Attributes: Only convert true, false & an empty string #5451
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
|
Size comparison: The PR - if this approach is accepted - still needs unit tests to confirm the new behavior. It is a bit reassuring that it didn't break any of our existing tests. |
6cbaca2 to
2adc8e9
Compare
The HTML spec defines boolean attributes: https://html.spec.whatwg.org/#boolean-attributes that often correlate with boolean properties. If the attribute is missing, it correlates with the `false` property value, if it's present - the `true` property value. The only valid values are an empty string or the attribute name. jQuery tried to be helpful here and treated boolean attributes in a special way in the `.attr()` API: 1. For the getter, as long as the attribute was present, it was returning the attribute name lowercased, ignoring the value. 2. For the setter, it was removing the attribute when `false` was passed; otherwise, it was ignoring the passed value and set the attribute - interestingly, in jQuery `>=3` not lowercased anymore. The problem is the spec occasionally converts boolean attributes into ones with additional attribute values with special behavior - one such example is the new `"until-found"` value for the `hidden` attribute. Our setter normalization means passing those values is impossible with jQuery. This patch takes a conservative approach, with backwards compatibility in mind, and makes the following changes: 1. It explicitly only applies the logic to the few most popular attribute names. 2. The getter only normalizes the empty string to the lowercased name, returning other attribute values unchanged - it seems unlikely that the empty string value will ever mean anything else than the attribute name repeated. 3. The setter continues to treat `false` as mean attribute removal and `true` as setting the value to the attribute name. All the other inputs are aplied unchanged. With this change, all existing jQuery unit tests were already passing.
2adc8e9 to
3d618cb
Compare
|
We've decided against this approach; closing. |
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.
Summary
The HTML spec defines boolean attributes:
https://html.spec.whatwg.org/#boolean-attributes
that often correlate with boolean properties. If the attribute is missing, it correlates with the
falseproperty value, if it's present - thetrueproperty value. The only valid values are an empty string or the attribute name.jQuery tried to be helpful here and treated boolean attributes in a special way in the
.attr()API:falsewas passed; otherwise, it was ignoring the passed value and set the attribute - interestingly, in jQuery>=3not lowercased anymore.The problem is the spec occasionally converts boolean attributes into ones with additional attribute values with special behavior - one such example is the new
"until-found"value for thehiddenattribute. Our setter normalization means passing those values is impossible with jQuery.This patch takes a conservative approach, with backwards compatibility in mind, and makes the following changes:
falseas mean attribute removal andtrueas setting the value to the attribute name. All the other inputs are aplied unchanged.With this change, all existing jQuery unit tests were already passing.
Checklist