jquery/sizzle

Don't set attributes while querying

fregante opened this issue · 3 comments

While using Sizzle in a MutationObserver that watches for attributes, I found that it's changing attributes in my DOM during an operation that I expect to be "pure", and it's causing a loop.

sizzle/src/sizzle.js

Lines 336 to 346 in 20390f0

// We can use :scope instead of the ID hack if the browser
// supports it & if we're not changing the context.
if ( newContext !== context || !support.scope ) {
// Capture the context ID, setting it first if necessary
if ( ( nid = context.getAttribute( "id" ) ) ) {
nid = nid.replace( rcssescape, fcssescape );
} else {
context.setAttribute( "id", ( nid = expando ) );
}
}

Why is this necessary? Should the condition be changed to only happen in browsers that don't support :scope?

mgol commented

:scope can only be used if the context to run querySelectorAll is maintained. This is not the case for selectors starting with the sibling combinator - we need to query on the parent so we can’t rely on :scope there.

So why is Sizzle still making the change even for selectors that don't need that?

What I'm asking is to avoid doing this unless strictly necessary, which feels like it's not for the vast majority of selections.

mgol commented

Other selectors shouldn’t be doing that. If you have an example where that’s not the case, please submit a test case on a service like JS Bin using jQuery or Sizzle and we’ll reopen the issue.