Bug Tracker

Ticket #3778 (closed bug: fixed)

Opened 6 years ago

Last modified 3 years ago

selector matching issues

Reported by: balazs.endresz Owned by: john
Priority: low Milestone: 1.8
Component: selector Version: 1.4.4
Keywords: Cc:
Blocking: Blocked by:

Description

The  patch that prevents selectors to match inside square brackets causes some other problems:

  • If the value contains a dot and an opening bracket then the class selector regex will match the string after the dot:

input[name=".types["]

  • Also it won't match .foo in this expression:

.foo #bar\]

Change History

comment:1 Changed 6 years ago by balazs.endre

Possible solution to the latter: /(?![^\\\[]*\])(?![^\\\(]*\))/

But this way the class regex (and the others) will wrongly match if a backslash is present after a dot inside an attribute selector: ´[foo=.ba\r]´ (besides the opening brackets, as in the first issue)

comment:2 Changed 6 years ago by balazs.endresz

I've been thinking about the first issue and came up with a simple solution to detect if there's still an erroneous match inside an attribute selector. It doesn't impact speed - maybe only in these edge cases a bit - but the only problem is that it should be in the Sizzle.filter function, which isn't really the place where this should ideally happen. Here's a standalone code that demonstrates the logic:

var classregex = /\.((?:[\w\u0128-\uFFFF_-]|\\.)+)(?![^\\\[]*\])(?![^\\\(]*\))/;
var attrregex = /\[((?:[\w\u0128-\uFFFF_-]|\\.)+)\s*(?:(\S?=)\s*(['"]*)(.*?)\3|)\]/;
var expr = 'input[name=".types["]' //original example
var expr = "input[name='.types\\.asdf']";
 //this would match ".types" too beacause of the fix for `.foo #bar\]` in the previous comment
var match = classregex(expr);

//
// and in Sizzle.filter the following would go after 
// `if ( (match = Expr.match[ type ].exec( expr )) != null ) {`
//

var nextpos=match && match.index + match[0].length;
if( nextpos && expr.charAt( nextpos ).match(/[\[\(\\]/) ){
  var isattr=attrregex(expr);
  if( isattr && ( (isattr.index + isattr[0].length) > nextpos ) )
    console.log("false positive"); // continue;
}

comment:3 Changed 6 years ago by balazs.endresz

It's getting even more interesting :) I tried to apply the above but the chunker splits this selector into two:

input[name=".types["]

becomes

input and name=".types["]

Now looking at the chunker this part: [^ >+~,(\[]+ is interesting -- why does it have to split the selector by the openening braces? Removing (\[ seems to solve this but I guess they have some purpose...

Btw, looks like people are already running into this bug:  http://groups.google.com/group/jquery-en/browse_thread/thread/eefe67dca10c8163

comment:4 Changed 6 years ago by john

  • Version changed from 1.2.6 to 1.3
  • Milestone changed from 1.3 to 1.3.1

I moved the discussion of input[name="types["] style selectors to bug #3928.

comment:5 Changed 6 years ago by dmethvin

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

Per John in #3928, fixed in SVN rev [6143].

comment:6 Changed 6 years ago by balazs.endresz

  • Status changed from closed to reopened
  • Resolution fixed deleted

I reopen this because #3928 was just a related problem. The original issue comes up when there's an opening bracket and a dot too in the attribute filter beacause that causes the class filter to match inside it (and the same goes for colons and pseudos).

comment:7 Changed 5 years ago by ekauffma

I am still seeing this issue in 1.3.2.

It appears that as long as a left bracket occurs after a dot (even if it is not the first character in the string) the element will not be properly selected. For example:

div[name='a.b[c'] will fail to select

whereas:

div[name='a.b]c'] works fine...

comment:8 Changed 4 years ago by snover

  • Priority changed from major to blocker
  • Status changed from reopened to open
  • Version changed from 1.3 to 1.4.4
  • Milestone changed from 1.3.1 to 1.5

Confirmed.  test case

comment:9 Changed 4 years ago by john

  • Priority changed from blocker to low
  • Milestone changed from 1.5 to 1.next

Will check this out later, post-1.5.

comment:10 Changed 3 years ago by dmethvin

Confirmed in bug triage, but low priority.

comment:11 Changed 3 years ago by dmethvin

#5482 is a duplicate of this ticket.

comment:12 Changed 3 years ago by timmywil

  • Status changed from open to closed
  • Resolution set to fixed
  • Milestone changed from 1.next to 1.8

valid if escaped:  http://jsfiddle.net/yjCW7/1/

Note: See TracTickets for help on using tickets.