[Bugfix:Submission] Fix BMP for file display#12623
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12623 +/- ##
============================================
- Coverage 21.64% 21.64% -0.01%
- Complexity 9654 9655 +1
============================================
Files 268 268
Lines 36244 36246 +2
Branches 487 487
============================================
Hits 7845 7845
- Misses 27916 27918 +2
Partials 483 483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
itsjanajana
changed the title
dagemcn
requested changes
Mar 24, 2026
dagemcn
left a comment
dagemcn
left a comment
Member
There was a problem hiding this comment.
The changes look good, the fix works as intended. Address failing CIs and I'll approve, they should be fairly easy fixes.
github-project-automation
Bot
moved this from Seeking Reviewer
to Work in Progress
in Submitty Development
itsjanajana
changed the title
Contributor
Author
|
@dagemcn
|
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Done
in Submitty Development
GarvitKhandelwal31
pushed a commit
to GarvitKhandelwal31/Submitty
that referenced
this pull request
Apr 13, 2026
## Why is this Change Important & Necessary? _Fixes Submitty#6583 BMP image created from Windows Paint triggers download on Chrome/Edge_ BMP files created from WindowsPaint and other applications are in format of `image/x-ms-bmp`. But, Chrome and Edge expect the files in format of `image/bmp`. So, Chrome and Edge handle it incorrectly, and download the file. This prevents the image from appearing inline and makes grading difficult. This PR fixes BMP file rendering without changing any other image formats. ## What is the New Behavior? <img width="2878" height="1543" alt="Screenshot 2026-03-22 034541" src="https://github.com/user-attachments/assets/2a86ae35-cd51-4e78-97ee-bf34ae900d63" /> **Old Behavior:** BMP file is in image/x-ms-bmp format and instead of displaying the image inline: 1. The file gets downloaded 2. The viewer shows nothing inline <img width="2879" height="1483" alt="Screenshot 2026-03-22 034954" src="https://github.com/user-attachments/assets/6c8dab1e-3cb7-4c58-b649-8626717a622f" /> <img width="2877" height="1486" alt="Screenshot 2026-03-22 035752" src="https://github.com/user-attachments/assets/f7c85aab-711b-4bee-8480-8ad227d5d753" /> **New behavior:** 1. BMP files display inline in the browser 2. No downloads happen 3. Any MIME type containing "bmp" is normalized to image/bmp(As shown above, non-Paint BMP files also display correctly. I have verified and attached above) ## What steps should a reviewer take to reproduce or test the bug or new feature? 1. Login as student, Submit a BMP file on any gradeable that uploads files. I did Upload Only Gradeable 2. Login as TA, Open the BMP file in grading view. 3. Before the change, the BMP file will download 4. After the change, it will display inline. All BMP files, including ones that are not Paint app files. ## Automated Testing & Documentation No new automated tests added, It is a minor MIME fix. Change only in file MiscController.php No additional documentation changes I have manually tested in local environment ## Other information Is this a breaking change? No Does this PR include migrations to update existing installations? No Are there security concerns with this PR? No --------- Co-authored-by: dagemcn <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this Change Important & Necessary?
Fixes #6583 BMP image created from Windows Paint triggers download on Chrome/Edge
BMP files created from WindowsPaint and other applications are in format of
image/x-ms-bmp. But, Chrome and Edge expect the files in format ofimage/bmp. So, Chrome and Edge handle it incorrectly, and download the file. This prevents the image from appearing inline and makes grading difficult. This PR fixes BMP file rendering without changing any other image formats.What is the New Behavior?
Old Behavior:
BMP file is in image/x-ms-bmp format and instead of displaying the image inline:
New behavior:
What steps should a reviewer take to reproduce or test the bug or new feature?
Automated Testing & Documentation
No new automated tests added, It is a minor MIME fix. Change only in file MiscController.php
No additional documentation changes
I have manually tested in local environment
Other information
Is this a breaking change? No
Does this PR include migrations to update existing installations? No
Are there security concerns with this PR? No