#7533 closed bug (fixed)

Opened November 16, 2010 02:52PM UTC

Closed December 06, 2010 09:41PM UTC

Last modified March 09, 2012 08:35PM UTC

Problem with id containing a period

Reported by:	kiequoo	Owned by:	jitter
Priority:	high	Milestone:	1.5
Component:	selector	Version:	1.4.4
Keywords:	regression, needsreview	Cc:
Blocked by:		Blocking:

Description

When creating a jQuery object from a piece of html, if an id attribute within the HTML contains a period, this breaks the find command and it never returns a result. This is best seen in this example in jsFiddle: http://jsfiddle.net/2gHqc/ - when run with jQuery 1.4.4 the alert displays 0, in previous versions of jQuery it returns 1.

Attachments (0)

Change History (24)

Changed November 16, 2010 02:55PM UTC by boushley comment:1

And having a period in the id is valid according to the W3 "ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".")." http://www.w3.org/TR/html401/types.html#type-name

Changed November 16, 2010 11:42PM UTC by jitter comment:2

_comment0:	→ 1289951082922689
_comment1:	A possible start/idea for a fix could be what I tried [https://github.com/jitter/sizzle/commits/t7533 here]. Not sure if I catched all relevant characters in the regexp → 1290635147508976
_comment2:	The issue is a bit more complex, as in HTML 5 basically everything is allowed as ID token. \ \ A possible start/idea for a fix could be what I tried on this [https://github.com/jeresig/sizzle/pull/37 pull request]. \ \ Not sure if I catched all relevant characters in the regexp, please double check. I will update when necessary. → 1291383376157252
component:	unfiled → selector
keywords:	→ regression, needsreview
milestone:	1.5 → 1.4.5
priority:	undecided → high
status:	new → open

The issue is a bit more complex, as in HTML 5 basically everything is allowed as ID token.

~~A possible start/idea for a fix could be what I tried on this pull request.~~

Not sure if I catched all relevant characters in the regexp, please double check. I will update when necessary.

Changed November 22, 2010 12:25PM UTC by jitter comment:3

owner:	→ jitter
status:	open → assigned

Changed November 24, 2010 03:48PM UTC by addyosmani comment:4

#7619 is a duplicate of this ticket.

Changed November 24, 2010 09:48PM UTC by jitter comment:5

blocking:	→ 7620

(In #7620) Thanks for taking the time to contribute to the jQuery project by writing a bug report and providing a testcase!

reduced test case

jQuery has sometimes issues when the id attribute contains meta characters. Here it is triggered as jQuery internally uses the present id attribute to speed up the search. Check on #7533 which (once fixed) should also fix this bug.

Changed November 24, 2010 11:11PM UTC by anonymous comment:6

Having a non escaped period (.) maybe valid for the HTML attribute value but *not* the CSS selector. QSA is correct when it chokes on the non-escaped IDs. http://www.w3.org/TR/CSS21/syndata.html#value-def-identifier

Because jQuery is dynamically constructing selectors it bares the responsibility of escaping them in those instances.

Changed November 24, 2010 11:24PM UTC by jitter comment:7

Replying to [comment:6 anonymous]:

QSA is correct when it chokes on the non-escaped IDs.

And nobody claims otherwise. That's why this ticket got accepted as valid and has a patch pending.

Changed November 25, 2010 12:25AM UTC by jitter comment:8

blocking:	7620

Changed November 25, 2010 12:30AM UTC by jitter comment:9

#7620 is a duplicate of this ticket.

Changed November 25, 2010 02:40PM UTC by jitter comment:10

#7634 is a duplicate of this ticket.

Changed November 27, 2010 07:50PM UTC by jitter comment:11

#7643 is a duplicate of this ticket.

Changed November 30, 2010 11:32AM UTC by jitter comment:12

#7661 is a duplicate of this ticket.

Changed December 02, 2010 11:13AM UTC by jitter comment:13

#7680 is a duplicate of this ticket.

Changed December 03, 2010 01:37PM UTC by jitter comment:14

_comment0:	new [https://github.com/jeresig/sizzle/pull/42 pull request] with suggested solution by John Resig → 1291383452137017

new pull request with the solution suggested by John Resig

Changed December 06, 2010 09:41PM UTC by john comment:15

resolution:	→ fixed
status:	assigned → closed

Landed.

Changed December 09, 2010 08:58PM UTC by jitter comment:16

#7740 is a duplicate of this ticket.

Changed December 14, 2010 11:01AM UTC by jitter comment:17

#7776 is a duplicate of this ticket.

Changed December 19, 2010 06:28PM UTC by jitter comment:18

#7807 is a duplicate of this ticket.

Changed December 21, 2010 07:06PM UTC by ajpiano comment:19

#7820 is a duplicate of this ticket.

Changed December 22, 2010 09:14AM UTC by ajpiano comment:20

#7829 is a duplicate of this ticket.

Changed December 22, 2010 09:33AM UTC by anonymous comment:21

Replying to [comment:20 ajpiano]: