Opened 12 years ago
Closed 12 years ago
#8539 closed bug (fixed)
Sizzle cache collision in browsers without querySelectorAll
| Reported by: | jryans | Owned by: | Timmy Willison |
|---|---|---|---|
| Priority: | high | Milestone: | 1.7 |
| Component: | selector | Version: | 1.5.1 |
| Keywords: | 1.7-discuss | Cc: | |
| Blocked by: | Blocking: |
Description (last modified by )
In browsers without querySelectorAll (IE 6, IE 7, FF 3.0), Sizzle marks the elements it encounters as seen for a particular selection run. The value used as a marker is an integer that comes from an internal counter which is incremented before each new selection, and these markers on left on the elements after selection completes.
This causes trouble when there are two copies of Sizzle on the page. Running a selection in Sizzle copy A sets the marker on a set of elements to 0. If you then run a selection in Sizzle copy B that passes through the same elements, Sizzle will assume it is safe to use a cached value at that point, since the stored marker of 0 matches copy B's marker for this run. This gives unexpected selection results.
See http://jsfiddle.net/uz4dt/6/ for a test case. The second test fails for browsers without querySelectorAll.
Change History (18)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
Updating: https://github.com/jquery/sizzle/pull/53
(not reviewed)
comment:3 Changed 12 years ago by
| Owner: | set to john |
|---|---|
| Priority: | undecided → blocker |
| Status: | new → assigned |
comment:4 Changed 12 years ago by
| Component: | unfiled → selector |
|---|
comment:6 Changed 12 years ago by
| Description: | modified (diff) |
|---|
+1, Seems like a bug, should be fixed
comment:7 Changed 12 years ago by
+1, Sizzle should have a mean to specify which counter field name to use, right?
comment:9 Changed 12 years ago by
| Description: | modified (diff) |
|---|
+1, although it would be preferable to ban IE6/7 from the planet.
comment:10 Changed 12 years ago by
| Description: | modified (diff) |
|---|
+1, Not 100% keen on the currently proposed solution but yeah, seems like we can do something here.
comment:14 Changed 12 years ago by
| Milestone: | 1.next → 1.7 |
|---|
comment:15 Changed 12 years ago by
| Owner: | changed from john to Timmy Willison |
|---|---|
| Priority: | blocker → high |
comment:16 Changed 12 years ago by
http://jsfiddle.net/timmywil/uz4dt/16/show/
The property on the element is just elem.sizcache. I'm thinking that should be like jQuery's expando, e.g. sizzle340987234098230. Then done can stay a regular counter and we avoid collisions.
comment:17 Changed 12 years ago by
oh, that's still linking to my local jquery. That can be changed to another jquery location if you'd like to see it working.
comment:18 Changed 12 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Update sizzle; Add sizzle cache collision iframe test. Fixes #8539.
Changeset: 8723f3b9e1db5d4fdcd50624441fe3536d1ae2f2
https://github.com/jeresig/sizzle/pull/53