Opened 13 years ago
Closed 13 years ago
#5505 closed bug (fixed)
hasClass doesn't work with carriage return or tab
Reported by: | batiste | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 1.4 |
Component: | core | Version: | 1.4a1 |
Keywords: | Cc: | [email protected]… | |
Blocked by: | Blocking: |
Description
className attribute in a element could contain carriage return or tab. jQuery hasClass doesn't work properly with them.
Here is a complete fix, with some doc, and tests integrated to the suite:
http://github.com/batiste/jquery
The performance impact should remain small because I just added a variable declaration and 2 replace call.
Attachments (1)
Change History (16)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
This is not a bug. According to standards class names should not contain tabs or carriage returns.
comment:3 Changed 13 years ago by
pbcomm: Reference please?
I have made my homework :
http://www.w3.org/TR/html401/struct/global.html#h-7.5.2
And try to validate that :
<!DOCTYPE html> <html> <body> <p class="toto tata">
comment:4 Changed 13 years ago by
http://www.w3.org/TR/html4/sgml/dtd.html#coreattrs
class CDATA #IMPLIED -- space-separated list of classes --
I usually use /\bclassName\b/ to check it's presence but it might be slower than an much simpler indexOf like it done at the moment. \b handles a class attributes containing tabs or carriage returns.
comment:5 Changed 13 years ago by
This is perfectly valid, working HTML. Css apply on these classes. jQuery hasClass implementation is incorrect. Please fix it.
<!DOCTYPE html> <html> <body> <p class="toto tata">
comment:6 Changed 13 years ago by
A more precise explanation, className is a cdata type. What the reference tells bout cdata type:
http://www.w3.org/TR/html4/types.html#type-cdata
CDATA is a sequence of characters from the document character set and may include character entities. User agents should interpret attribute values as follows: 1. Replace character entities with characters, 2. Ignore line feeds, 3. Replace each carriage return or tab with a single space.
The User agent interpret the attribute value like that. And jQuery need to do the same thing. The attribute stay unchanged by the browser so jQuery has to do the same work too.
comment:7 Changed 13 years ago by
Add a test for dotted class name :
http://github.com/batiste/jquery/commit/ca1c88e3884ef3d234f2d6a935aa7390f540a7e7
comment:8 Changed 13 years ago by
comment:9 Changed 13 years ago by
See also #3680; browsers may trim leading/trailing space in the className CDATA before it gets to jQuery.
comment:10 Changed 13 years ago by
I couldn't find an explicit W3C reference to newlines in class attributes, other than the one I mentioned in #3680. The section batiste referenced says, "For some HTML 4 attributes with CDATA attribute values, the specification imposes further constraints on the set of legal values for the attribute that may not be expressed by the DTD."
In the section greut references, it says "Multiple class names must be separated by white space characters." Since CR and LF are white space it sounds like they should be allowed in className.
comment:11 Changed 13 years ago by
Changed 13 years ago by
Attachment: | hasClass.patch added |
---|
comment:12 Changed 13 years ago by
I said that my fix will help with #3680 but it's not true because I only fixed hasClass and removeClass.
I have not looked at all if there was a similar issue with selectors.
comment:13 Changed 13 years ago by
My patch is actually weird but seems proper to me...
Would be probably better to fix Sizzle?
For example something like that :
$('<div class="foo" />').hasClass('.foo');
Is seems to work with 1.3.2, but is debatable if it should work or not... This seems more correct:
$('<div class="foo" />').hasClass('foo');
comment:14 Changed 13 years ago by
Link to the latest commit:
http://github.com/batiste/jquery/commit/e379815c095cc971a37f751e2b7f73b4b09c694b
comment:15 Changed 13 years ago by
Component: | unfilled → core |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Version: | 1.3.2 → 1.4a1 |
Landed these fixes in two commits: http://github.com/jquery/jquery/commit/649024909d376032e6e9c41f209182d584e51043 http://github.com/jeresig/sizzle/commit/40a9507edae124fdce8fa8760592df1870ec27be
Thanks for the patches and test cases!
This bug is the same as :
http://dev.jquery.com/ticket/4761
It happens on every browser, no just IE.