A selector that begins or ends with a comma fails in 1.8 but works in 1.7.2

Reported by:	uniqname	Owned by:	uniqname
Priority:	undecided	Milestone:	None
Component:	unfiled	Version:	1.8.0
Keywords:		Cc:
Blocked by:		Blocking:

Description

In previous versions of jQuery (1.7.2 specifically) if a selector has a comma either at the beginning or end -- for instance if an empty array is joined and concatenated to a string -- the selector still matches correctly for other valid selectors in the list. This has failed to be the case in 1.8

Using jQuery 1.8, an error is thrown

http://jsfiddle.net/qARJ8/1/

Using jQuery 1.7.2 the selector parses correctly

http://jsfiddle.net/qARJ8/2/

Attachments (0)

Change History (5)

Changed August 14, 2012 03:55PM UTC by dmethvin comment:1

owner:	→ uniqname
status:	new → pending

But ... isn't the 1.8 behavior correct? A leading or trailing comma would seem to be a syntax error. For your specific case in the fiddle, you could have just pushed '[data-attr]' onto list_o_elements.

Changed August 14, 2012 04:51PM UTC by uniqname comment:2

_comment0:	Whether 1.8 behavior is correct is debatable. But it is different than 1.7.2 behavior and what "worked" before now fails rather inelegantly. My thought is that either Sizzle could trim commas from the ends or split the list of selectors on commas. If the jQuery method takes a comma separated list of selectors, it makes sense that it would work similarly to a string split on commas. After all $('') works just fine without throwing any errors. That could totally be a perception problem on my part though. → 1344963163550900
_comment1:	Whether 1.8 behavior is correct is debatable. But it is different than 1.7.2 behavior and what "worked" before now fails rather inelegantly. My thought is that either Sizzle could trim commas from the ends or split the list of selectors on commas. If the jQuery method takes a comma separated list of selectors, it makes sense that it would work similarly to a string split on commas. After all $("") works just fine without throwing any errors. That could totally be a perception problem on my part though. → 1344963237021379
status:	pending → new

Whether 1.8 behavior is correct is debatable. But it is different than 1.7.2 behavior and what "worked" before now fails rather inelegantly. My thought is that either Sizzle could trim commas from the ends or split the list of selectors on commas. If the jQuery method takes a comma separated list of selectors, it makes sense that it would work similarly to a string split on commas. After all $("") works just fine without throwing any errors. That could totally be a perception problem on my part though.

Also, getting around this issue is pretty easy, but it does mean it's not as plug and play as it could be.

Changed August 14, 2012 05:07PM UTC by dmethvin comment:3

_comment0:	> Whether 1.8 behavior is correct is debatable. \ \ Well, http://www.w3.org/TR/css3-selectors/#grouping says: \ \ > Groups of selectors: A comma-separated list of selectors ... \ \ That doesn't seem unambiguous to me. \ \ > If the jQuery method takes a comma separated list of selectors, it makes sense that it would work similarly to a string split on commas. \ \ `$('div, ul[data-list="a,b,c"], p')` \ \ We can't always preserve old bugs, and normally we try to fix them. At times that will cause code that previously "worked" in some way to break. But I don't think this is a case where we'd want to start rewriting invalid selectors by trimming off bits in the name of backcompat. \ → 1344966597051707
resolution:	→ invalid
status:	new → closed

Whether 1.8 behavior is correct is debatable.

Well, http://www.w3.org/TR/css3-selectors/#grouping says:

Groups of selectors: A **comma-separated** list of selectors ...

That doesn't seem ambiguous to me.

If the jQuery method takes a comma separated list of selectors, it makes sense that it would work similarly to a string split on commas.

$('div, ul[data-list="a,b,c"], p')

We can't always preserve old bugs, and normally we try to fix them. At times that will cause code that previously "worked" in some way to break. But I don't think this is a case where we'd want to start rewriting invalid selectors by trimming off bits in the name of backcompat.

Changed August 14, 2012 05:20PM UTC by Uniqname comment:4

'$('div, ul[data-list="a,b,c"], p')

Touché

Changed August 14, 2012 06:13PM UTC by ajpiano comment:5

I'm gonna throw in my 2c here to say that it is probably a perception problem. From a code-sanity perspective, it would make a lot more sense to push your addition class onto the array, and then join the whole thing with ", ", rather than join the array first, and concat a string with a leading comma onto that. The fact that the input worked before doesn't make the reasoning behind doing it like that any more sound, and occasionally we do have to break strange edge cases that used to "work" in order to solve real bugs, as is the case here.

Side navigation

#12298 closed bug (invalid)