239079 - ID tokens starting with digit allowed by CSS scanner

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Eduardo Mercovich]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20031007 Firebird/0.7
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5) Gecko/20031007 Firebird/0.7

div.600 {
 width: 600px; 
}

Doesn't render in 600px wide. 

div#600 {
 width: 600px; 
}

Works OK. 

Reproducible: Always
Steps to Reproduce:
1. Create an HTML document with a <div class="600">.
2. Link to it a stylesheet with the selector/prop: div.600 {width: 600px; }
3. Watch the page. The div flows normally, and it is not 600px wide.
4. Change the <div class="600"> to <div id="600">.
5. Watch the page. Now the div in the document is 600 px wide.
Actual Results:  
The div in the document is 600 px wide, asi it should be. 

Expected Results:  
Render the div block with 600 px width from the first attempt (with the CLASS
selector). 

No crash.

Comment 1

[Bill Mason]

A class or ID starting with a digit is invalid.
http://www.w3.org/TR/2004/CR-CSS21-20040225/syndata.html#q6

You're probably writing a quirks-mode document where breaking that rule is
allowed for backwards compatibility.  Probably quirks mode allows it in IDs but
not classes.

A testcase or URL showing this problem would help.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Actually, this is a problem in both modes.  The code at
http://lxr.mozilla.org/seamonkey/source/content/html/style/src/nsCSSScanner.cpp#485
doesn't check that the first char of the putative ID is a StartsIdent().

It probably should, but deferring to David on this one, since syntax is his thing...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: So like this, I guess

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 145301 [details] [diff] [review]
So like this, I guess

No, it should still tokenize as ID (which should probably be HASH), it should
shouldn't parse as an ID selector.

Doesn't this patch break colors?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er, yes.  Thinko... yes, a better name would help.  ;)

Taking.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: So more like this....

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145304 [details] [diff] [review]
So more like this....

I preferred this approach to duplicating the logic in nsCSSParser, but I could
do that too, I guess.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145304 [details] [diff] [review]
So more like this....

One other option is to make the scanner produce either HASH or ID tokens
depending on whether the thing following '#' is an ident and then change color
parsing to work with both token types while leaving id parsing to only work
with the ID token.

Comment 9

[Hixie (not reading bugmail)]

(While you're at it should we remove IS_LATIN1? It doesn't seem to ever be used,
and it seems useless... Then again, what would I know!)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yeah, that can probably go.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145304 [details] [diff] [review]
So more like this....

I totally failed to set those flags right... David, see comment 8

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Same without IS_LATIN1

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145374 [details] [diff] [review]
Same without IS_LATIN1

Could this be approved for 1.7?  This fixes a bug in our ID selector parsing in
CSS (we're allowing invalid selectors).

Comment 14

[chris hofmann]

Comment on attachment 145374 [details] [diff] [review]
Same without IS_LATIN1

a=chofmann for 1.7

Comment 15

[Boris Zbarsky [:bzbarsky]]

Checked in, though I had to check in a DEBUG bustage fix as well as a Windows
one (can't init static members at the declaration point in MSVC).

Note that the fix is that now ID and class selectors act identically and do NOT
apply the styles (as class selectors already did).

Comment 16

[Anne (:annevk)]

*** Bug 256684 has been marked as a duplicate of this bug. ***

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This fix is actually incorrect; it should have happened before character escape
processing, not after.  See bug 299148.