Bug 14373

Summary: CSS1: selectors (classes and IDs) cannot start with a dash or with a digit
Product: WebKit Reporter: Gérard Talbot (no longer involved) <browserbugs2>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bugs
Priority: P2    
Version: 523.x (Safari 3)   
Hardware: PC   
OS: Windows XP   
URL: http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/CSS1ForwardCompatibleParsing.html
Attachments:
Description Flags
first attempt
none
Now with testcase mjs: review+

Gérard Talbot (no longer involved)
Reported 2007-06-24 16:45:42 PDT
"in CSS1, selectors (element names, classes and IDs) can contain only the characters A-Z, 0-9, and Unicode characters 161-255, plus dash (-); they cannot start with a dash or a digit;" CSS 1, section 7.1 Forward-compatible parsing http://www.w3.org/TR/REC-CSS1#forward-compatible-parsing "In CSS, identifiers (including element names, classes, and IDs in selectors) can contain only the characters [a-z0-9] and ISO 10646 characters U+00A1 and higher, plus the hyphen (-) and the underscore (_); they cannot start with a digit, or a hyphen followed by a digit." CSS 2.1, section 4.1.3 Characters and case http://www.w3.org/TR/CSS21/syndata.html#q6 Steps: Load provided URL Actual results: the 3rd sentence is green; 1st, 2nd and 4th sentence are red; Expected results: all 4 sentences should be green Notes: - MSIE 7 and Firefox 2.0.0.4 render 2 green sentences and 2 red sentences - Opera 9.21 renders 4 green sentences - This bug spun from bug 14345 - Safari 3.0.2 build 522.13.1 here Other testcases: http://www.editions-eyrolles.com/css2/tests/syntaxe/syntax5.htm (look at the 5th, last list item) http://www.hixie.ch/tests/evil/css/css21/tests/t0509-id-sel-syntax-01-f.htm
Attachments
first attempt (26.81 KB, patch)
2007-06-26 12:58 PDT, Rob Buis
no flags
Now with testcase (139.10 KB, patch)
2007-06-27 11:08 PDT, Rob Buis
mjs: review+
Gérard Talbot (no longer involved)
Comment 1 2007-06-24 16:46:36 PDT
*** Bug 14345 has been marked as a duplicate of this bug. ***
Gérard Talbot (no longer involved)
Comment 2 2007-06-25 11:55:59 PDT
Latest CSS 2.1 spec seems to suggest that identifiers can start with a hyphen (-) followed by an alphabetic character [A-Za-z]. Also, CSS 2.1, section G.3 Comparison of tokenization in CSS 2.1 and CSS1 http://www.w3.org/TR/CSS21/grammar.html#tokenizer-diffs says { In CSS1, a class name could start with a digit (".55ft") } which is not true. Finally, HTML 4.01 claims that { 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/html4/types.html#type-id
Rob Buis
Comment 3 2007-06-26 12:58:37 PDT
Created attachment 15257 [details] first attempt This introduces no new shift/reduce conflicts. I have tried it on the feature_branch where it improved two testcases (one was in the bug report) and no regressions. However, someone may want to try it on ToT, maybe check for speed regression and check whether some more elegant flex/bison approach can be taken ( I am not an expert on those tools yet). Cheers, Rob.
Gérard Talbot (no longer involved)
Comment 4 2007-06-26 18:45:21 PDT
Rob, I've redone the testcase to be easier to read code (more explicit, self-explanatory code) and text, figure out interactively speaking. Only the 1st sentence, with <p id="1stSentence">First sentence ...</p> can be safely fixed by WebKit. Safari 3.0.2 renders the 1st sentence as red when all other good web standards compliant browsers (Opera 9.21, MSIE 7, Firefox 2.0.0.4, Amaya 9.54) render it green. All other 4 cases/sentences depend on CSS 2.1 spec which is still not final, definitive. CSS 2.1 25 February 2004 WD introduced exceptions and IMO unneeded complexity.
Maciej Stachowiak
Comment 5 2007-06-26 21:27:05 PDT
Rob, which test cases does the patch change (in the test case linked in the URL field)? I think we should only change the first sentence to be green, since the other existing behavior matches IE and Firefox and is correct per CSS 2.1. It would help if you added that test case and its new expected results to LayoutTests in your patch. It looks like the patch will actulaly do the right thing, since ident allows a leading dash, but rules out leading digits or digits after the dash, but then maybe the bug should be retitled.
Dave Hyatt
Comment 6 2007-06-26 21:59:05 PDT
One concern I have here is that I vaguely recall us supporting this on purpose for some reason...
Maciej Stachowiak
Comment 7 2007-06-26 23:10:02 PDT
It's unlikely to be a compatibility issue if we match what Firefox 2 and IE7 do.
Rob Buis
Comment 8 2007-06-27 01:32:01 PDT
Hi, (In reply to comment #5) > Rob, which test cases does the patch change (in the test case linked in the URL > field)? I think we should only change the first sentence to be green, since the Only the first sentence changed, so now it matches FF. > other existing behavior matches IE and Firefox and is correct per CSS 2.1. It > would help if you added that test case and its new expected results to > LayoutTests in your patch. It looks like the patch will actulaly do the right > thing, since ident allows a leading dash, but rules out leading digits or > digits after the dash, but then maybe the bug should be retitled. Is LayoutTests/css2.1 a good place for it? I can do that and post a new patch, but feel it is better to do one for ToT then. Cheers, Rob.
Rob Buis
Comment 9 2007-06-27 01:34:57 PDT
Hi Gérard, (In reply to comment #4) > Rob, I've redone the testcase to be easier to read code (more explicit, > self-explanatory code) and text, figure out interactively speaking. > > Only the 1st sentence, with > <p id="1stSentence">First sentence ...</p> > can be safely fixed by WebKit. Safari 3.0.2 renders the 1st sentence as red > when all other good web standards compliant browsers (Opera 9.21, MSIE 7, > Firefox 2.0.0.4, Amaya 9.54) render it green. > > All other 4 cases/sentences depend on CSS 2.1 spec which is still not final, > definitive. CSS 2.1 25 February 2004 WD introduced exceptions and IMO unneeded > complexity. I don't follow the css spec, but my patch does fix that first sentence of your testcase, and leaves the rest as it was. Cheers, Rob.
Rob Buis
Comment 10 2007-06-27 11:08:47 PDT
Created attachment 15273 [details] Now with testcase I stripped the main testcase linked to the bug report a bit and attached it in the patch, with test results. Cheers, Rob.
Maciej Stachowiak
Comment 11 2007-06-28 01:45:18 PDT
Comment on attachment 15273 [details] Now with testcase r=me Hyatt thinks there might be Apple-specific dependencies on the old behavior but I think we should bite the bullet and see what happens.
Rob Buis
Comment 12 2007-06-28 09:01:54 PDT
Landed in 23854.
Note You need to log in before you can comment on or make changes to this bug.