[ImageResizer] Add support for blanks in height/width fields (#15306) #15368
Conversation
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
jaimecbernardo
approved these changes
Jan 17, 2022
Contributor
jaimecbernardo
left a comment
jaimecbernardo
left a comment
There was a problem hiding this comment.
LGTM! Thank you for the contribution!
SeraphimaZykova
approved these changes
Jan 18, 2022
Contributor
SeraphimaZykova
left a comment
SeraphimaZykova
left a comment
There was a problem hiding this comment.
Tested, works as expected. Nice work!
Contributor
|
Thank you for the contribution! |
CleanCodeDeveloper
deleted the
fix/image-resizer-#15306-Support-blanks-for-width-and-height-
branch
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.
Summary of the Pull Request
What is this about:
Fix auto width/height feature that it works again as described:
Using a blank in the width/height field causes the image resizer to crash as described in #15306.
My guess is that this was a regression from the
Newtonsoft.JsontoSystem.Json.Textmigration for JSON Settings serialization (if that ever happened, just guessing). Unfortunately,System.Json.Texthandles cases likeNaNdifferently thanNewtonsoft.Json. Support for native support for "Named Floating Point Literals" where added with version 5 ofSystem.Text.Json. See Microsoft Docs for further details.I first thought that we need to wait for the .NET 6 migration to fix this due to the need for v5
System.Text.Json. Turns out that we already usingSystem.Text.Jsonv5.0.2 in ImageResizer so here’s the fix.What is included in the PR:
Added JsonSerializerOptions in the settings serialization class of image resizer.
How does someone test / validate:
Quality Checklist
Contributor License Agreement (CLA)
A CLA must be signed. If not, go over here and sign the CLA.