Bug Tracker

Ticket #8539 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

Sizzle cache collision in browsers without querySelectorAll

Reported by: jryans Owned by: timmywil
Priority: high Milestone: 1.7
Component: selector Version: 1.5.1
Keywords: 1.7-discuss Cc:
Blocking: Blocked by:

Description (last modified by ajpiano) (diff)

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

comment:2 Changed 3 years ago by rwaldron

comment:3 Changed 3 years ago by rwaldron

  • Owner set to john
  • Priority changed from undecided to blocker
  • Status changed from new to assigned

comment:4 Changed 3 years ago by john

  • Component changed from unfiled to selector

comment:5 Changed 3 years ago by john

  • Keywords 1.7-discuss added

Nominating ticket for 1.7 discussion.

comment:6 Changed 3 years ago by rwaldron

  • Description modified (diff)

+1, Seems like a bug, should be fixed

comment:7 Changed 3 years ago by jaubourg

+1, Sizzle should have a mean to specify which counter field name to use, right?

comment:8 Changed 3 years ago by timmywil

+1,

comment:9 Changed 3 years ago by dmethvin

  • Description modified (diff)

+1, although it would be preferable to ban IE6/7 from the planet.

comment:10 Changed 3 years ago by john

  • Description modified (diff)

+1, Not 100% keen on the currently proposed solution but yeah, seems like we can do something here.

comment:11 Changed 3 years ago by scott.gonzalez

+1

comment:12 Changed 3 years ago by addyosmani

+1

comment:13 Changed 3 years ago by ajpiano

  • Description modified (diff)

+1

comment:14 Changed 3 years ago by dmethvin

  • Milestone changed from 1.next to 1.7

comment:15 Changed 3 years ago by timmywil

  • Owner changed from john to timmywil
  • Priority changed from blocker to high

comment:16 Changed 3 years ago by timmywil

 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 3 years ago by timmywil

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 3 years ago by timmywil

  • Status changed from assigned to closed
  • Resolution set to fixed

Update sizzle; Add sizzle cache collision iframe test. Fixes #8539.

Changeset: 8723f3b9e1db5d4fdcd50624441fe3536d1ae2f2

Note: See TracTickets for help on using tickets.