132785 - Fault in formatting of webpage; support <!-- comment --X>

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P1)

Description

[bruno]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9+)
Gecko/20020321
BuildID:    2002032103

Formatting on page does not appear correctly --- I beLIEve it has something to
do with CSS...

Reproducible: Always
Steps to Reproduce:
1.Open Mozilla
2. enter URL: http://www.xenu.net/
3. compare with IE 5.5 or 6

Actual Results:  The page doesn't appear correctly -- several things missing
with formatting and content....

Expected Results:  Correct formatting and content should appear

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Basically, in strict comment parsing 

<!-- comment --X> 

where X is any non-whitespace character is treated as an underminated comment. 
This seems wrong to me....

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Ok, this is another regression caused by my comment fixing. The question in
here, as well as in bug 132238, is: should we really fix them in strict mode
comment parsing? For both of these cases the fix would be rather simple.

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix

Relaxed IsGoodCommentEnd() function so that it accepts anything, excluding two
consecutive dashes "--" because that indicates we must continue with regular
processing. Also renamed the function to IsCommentEnd().

Comment 5

[harishd]

Comment on attachment 75636 [details] [diff] [review]
Proposed fix

r=harishd

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 75636 [details] [diff] [review]
Proposed fix

sr=jst

Comment 7

[rubydoo123]

just looked at the SGML Handbook, the way I interpret the Comment Declation 
[section 10.3 SGML Handbook] is that you can have any of these scenarios and be 
legal:

<!>
<!-- blah -->
<!-- blah -- >
<!-- blah -- -- blah -->
<!-- blah ---- blah -->
<!-- blah -- -- blah -- >

there may be more though!

You cannot have: <! >

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Yes, in SGML. But this is HTML, which has forbidden a lot of SGML stuff, and we
don't have a validating HTML parser.

Anyway, the question is what should we do when we see these in strict mode:

This bug:
<!--foo--bar>

bug 132238:
<!>
<!foo>
<! >

IE accepts all of them. I am getting more convinced that we should too. I have
fixes for these as well.

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Bug 110544 was the one that caused this.

Comment 10

[rubydoo123]

Yes, I realize we are HTML. However, the comment declartion parameters set forth 
 in SGML is pretty standard, even in HTML. My recommendation is if the comment 
syntax falls out of the syntax set in the SGML declaration, then handle it like 
any other malformed element. If I recall correctly, malformed comments resulted 
in the comment being rendered.

As for IE handling poorly constructed elements, so what? If the IE folks want to 
go down that path, let them. Wasn't our motto "standards"?

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

NS 4.x also happily eats bad comments. And since it took only 7 days for this
regression to be found I suspect there are a lot of broken pages out there. More
reasons why I believe we must fix these regressions in the code, and not by
evangelism.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 75636 [details] [diff] [review]
Proposed fix

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Checked in.

Comment 14

[Christopher Hoess (gone)]

Heikki: all this patch does is mess up strict mode.  The testcase already works
in quirks mode.  Given that we evangelized multi-dash comments for strict mode
documents (which I would estimate to be at least an order of magnitude more
common than any of these), this patch is pointless and should be backed out.

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

We can experiment all we want after 1.0, but I don't like to break things now.