fix: support non-canonicalised srcset attribute name #2958 #2968
Conversation
This file contains 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
gg-kialo
temporarily deployed
to
authentication
GitHub Actions
Inactive
|
fyi: both locally and in github codespaces I've been unable to get the tests to run. After running I tried adding adding a test but since removed it since it was failing. I don't think it was testing the right thing. |
gg-kialo
temporarily deployed
to
authentication
GitHub Actions
Inactive
gg-kialo
temporarily deployed
to
authentication
GitHub Actions
Inactive
|
I've pushed with an added test. I managed to figure how to get them to run locally (you have to open the qunit server 🤦). fyi, I'll be OoO until Tuesday. |
Aleksey28
requested changes
Oct 20, 2023
Some frameworks, like React, use non-canonicalised attribute names. The spec and browsers allow this, but hammerhead is only checking if the attribute name without canonicalisation matches `srcset`. By using the `loweredAttr` for the comparison, hammerhead will be more accepting of inputs and resolve srcset issues with React projects (fixes DevExpress#2958). See facebook/react#10863 for more background.
gg-kialo
temporarily deployed
to
authentication
GitHub Actions
Inactive
Aleksey28
approved these changes
Oct 24, 2023
|
Release v31.6.3 addresses this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Some frameworks, like React, use non-canonicalised attribute names. The spec and browsers allow this, but hammerhead is only checking if the attribute name without canonicalisation matches
srcset.By using the
loweredAttrfor the comparison, hammerhead will be more accepting of inputs and resolve srcset issues with React projects (fixes #2958).See facebook/react#10863 for more background.
Purpose
Fixes #2958 to better support
srcSetattributes in React applications.Approach
Canonicalises the attribute name before comparing to
srcset.References
Related react issue (and there are various duplicates of this) - facebook/react#10863
Pre-Merge TODO