37448 - [regression]HTML comment in STYLE block can cause style rule to be skipped

Status: VERIFIED INVALID

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

Description

[Teruko Kobayashi]

At this page, the first <p class="mincyou" does not work.
It does not pick up the Fonts and color style.

I have same test case written in Euc-jp in 
http://babel/tests/browser/css/css2/jFonts-eucjp.html
This works fine.

Tested 2000042709 Win32 with Winnt 4.0J.
This worked fine in 20000426 build.

Comment 1

[Teruko Kobayashi]

Corrected URL.

Comment 2

[Frank Tang]

I don't think this is realated to @charset at all. The first paragraph are used 
the interneal stylesheet, right ? It is the 2nd one use the exteran css, right ? 
It is a bug , but not related to @charset.

Comment 3

[Marc Attinasi]

I think you're correct, Frank. I was not seeing the bug yesterday on my local 
tree, however I had not pulled the recent string changes yet. I am doing that 
now so I can look into what went wrong. I am pretty sure the problem is outside 
of style but I will spend some time looking into it anyway since I have made 
changes recently to deal with alternate charsets in the CSSParser.

Comment 4

[Marc Attinasi]

I tracked this down to a problem in the CSSParser/CSSScanner. The comment 
token in the <style> block is <!--- instead of <!-- and the extra '-' is causing 
the rule immediatly following the comment to be skipped, hence the invalid 
appearance. This is not an I18L issue at all...

Backing out dBaron's changes to nsCSSParser and nsCSSScanner causes it to work 
again. As per conversation with Frank, reassigning to dbaron.

Comment 5

[Marc Attinasi]

Attachment: Testcase showing the problem with the extra dash in a comment

Comment 6

[Marc Attinasi]

Updated title to reflect the real problem

Comment 7

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

An <!--- comment is illegal CSS.  The CSS grammar says that <!-- and --> should
be ignored, but nothing else.  If there are real pages on the web that use
something other than <!-- and -->, then we should put the old code back for
quirks mode only.  Has anybody seen pages out on the web that do this?

Comment 8

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

Any opinions on what should happen here?  I think there are two options:
 * mark this bug INVALID
 * change the CDO ("<!--") tokenization *for quirks mode* back to the way it
used to be (any number of dashes)

Comment 9

[Marc Attinasi]

I think this should either be INVALID or we should support a friendlier approach 
in all modes. NavQuirks is out of the question because Nav does not handle it 
either, though IE does. 

The friendlier approach would be complicated to implement, but it would make the 
most sense to authors. Namely, if there is other junk after the SGML comment 
delimiter, then eat it up until a newline or a valid style rule. This way 
somebody could write:

<style>
<!-- BLOCK FROM OLD AGENTS --
H1 H2 H3 H4 H5 H6 { color: lime; }
-->
</style>

and they could write:

<style>
<!-- H1 H2 H3 H4 H5 H6 { color: lime; } -->
<!-- H1 H2 H3 H4 H5 H6 { font-weight: bold; } -->
</style>

IE supports both of these correctly.

The CSS2 spec is not very specific on this, at least from what I read in 4.1.9 -  
I do not think we would be violating the spec by supporting the comments as I 
described above. The spec is just trying to provide a mechanism for hiding the 
style rules. That said, I don't see this as important so I would just close it 
as INVALID.

Comment 10

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

It would be violating the spec to support anything other than <!-- and -->.  The
spec is very specific.  See the third paragraph of 4.1.7 and all of 4.2.

The spec is very specific about error handling because, on the web, authors tend
to design for browsers, rather than for specs.  If it works in a browser, then
authors figure it should work, and other browsers are then forced to handle
those pages.  This creates an escalating leniency (which becomes very hard to
code) from release to release of browsers.  (Consider the HTML parser as an
example.)  The authors of CSS didn't want CSS to go the way of HTML.

If this is used on the web, and pages depend on it, then quirks mode should
support it.  However, since, as you correctly point out, Nav 4.x correctly
requires the comments to be <!-- and --> (but also incorrectly requires them to
be "nested" correctly), I'm marking this bug INVALID.

Comment 11

[Hixie (not reading bugmail)]

David is absolutely right.

Comment 12

[Jesse Ruderman]

Closely related, also "invalid": bug 37448

Comment 13

[Jesse Ruderman]

Oops, bug 37995.

Comment 14

[KenW]

This bug has reappeared in build 2000071910. The (legal) comment causes Mozilla 
to ignore the first style rule.

Comment 15

[KenW]

Attachment: example of legal comment hosing next style rule

Comment 16

[Marc Attinasi]

The comment in the testcase is not legal for a style sheet: see David Baron's 
comments of 2000-05-03 12:02 for clarification.

Comment 17

[ken_warkentyne]

The comment in the test case is legal. There are only two dashes in <!-- and --> 
strings. It works in Netscape 4 and IE 5.5 (only the colours, they don't do the 
outlines).

Comment 18

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

No, it's not legal.  <!-- and --> are *not* comment delimeters in CSS.  They are
ignored within CSS stylesheets, so that you can do:

<style type="text/css">
<!--
p { color: green; }
/* Note:  this will not show up in old browsers */
-->
</style>

Therefore your stylesheet is equivalent to what would be there if you removed
the "<!--" and "-->" but not the text in between.