Side navigation
#13539 closed feature (fixed)
Opened March 01, 2013 01:06AM UTC
Closed April 05, 2013 05:16AM UTC
Stop overriding Sizzle.attr
| Reported by: | gibson042 | Owned by: | |
|---|---|---|---|
| Priority: | low | Milestone: | |
| Component: | attributes | Version: | 2.0b2 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
jQuery.attr replaces Sizzle.attr and adds value in the following ways:
1. safely no-ops on non-element input
2. falls back to jQuery.prop in the absence of getAttribute
3. sets an attribute when value is defined
4. allows jQuery.attrHooks to override standard getters/setters for HTML attributes
5. normalizes booleans, value (''qua'' defaultValue), and oldIE attroperty confusion
However, replacing Sizzle.attr comes with some consequences:
- Sizzle uses it instead of its own function for resolving selectors
- Code is duplicated between the two libraries
- Several fixes that should have made it to Sizzle are not there because we patched jQuery
I'd like to stop overriding the Sizzle function and instead let jQuery defer to it for getting attribute values. This will necessitate moving the jQuery fixes into Sizzle (point 5), and will also mean that .is and .attr can disagree if someone defines a get attrHook (or alternatively, that we should do away with get hooks).
Does this idea have legs?
Attachments (0)
Change History (5)
Changed March 01, 2013 01:24AM UTC by comment:1
Changed March 01, 2013 01:29AM UTC by comment:2
| component: | unfiled → attributes |
|---|---|
| milestone: | None |
| priority: | undecided → low |
2.x has no get attrHooks, so this should just mean tweaks in Sizzle and deletes in the 1.x attributes module.
Changed March 02, 2013 07:20PM UTC by comment:3
Maybe this has legs for 2.x, but not for 1.x. Sizzle's attribute handling is not meant to be comprehensive. It was designed with the intention that any library could override it and fill in all of the edge cases. Sizzle.attr is small. It should stay small and separating them in jQuery doesn't mean less duplicated code, it means more. I'm not even sure it should be done for 1.x.
Changed March 02, 2013 07:28PM UTC by comment:4
| status: | new → open |
|---|---|
| version: | git → 2.0b2 |
I meant 2.x on that last. If Sizzle.attr has exactly the same results as jQuery.attr in 2.0 right now, then it would be possible to not use jQuery.attr. Still, the point on the duplicated code is in the wrong list. And as I was saying, points 1 and 3 are good things. As a side note, I'm not sure .is and .attr need to disagree. Use attrHooks at your own risk. A blindly-implemented custom attrHook will probably screw up .attr too.
It would be worth investigating how to clean that up. A lot of the hooks were put in for cases that we can hopefully put behind us. I don't think we should be encouraging anyone outside the jQuery team to add any hooks, they have too many implications.
How crazy does this get now that we have two actively maintained versions and two selector engine modules?