321484 - CRs (\r) in HTML comments should be removed by the parser

Status: RESOLVED FIXED

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

Description

[Uri Bernstein]

Currently, Carriage Returns (0x0d, \r) inside HTML comments are left intact by the HTML parser, and therefore may appear in comment nodes in the DOM.
According to bz (bug 319705 comment #12): "The DOM newline character is LF. All CRs should be removed or converted to LFs by the parser."

Comment 1

[Uri Bernstein]

Attachment: testcase

This testcase has comment lines ending with CRLF, CR, and LF. The content dump from this testcase is:

html@0x2220b2a0 refcount=5<
  head@0x206ea020 refcount=2<
    Text@0x206ee970 refcount=1<\n\n>
    Comment@0x206ee9f0 refcount=1<!--\nline with CRLF\r\nline with CR\rline with LF\nAnother line\n-->
  >
  body@0x206eea80 refcount=3<
    Text@0x206f23c0 refcount=2<\n\n>
  >
>

Notice the \r\n, \r, and \n.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, compare CTextToken::Consume and ConsumeUntil in nsHTMLTokens.cpp (both of which do the right thing with CR) with CCommentToken::Consume (which does not).

Perhaps we should push the CR handling up to a higher level in the scanner so that individual tokens don't have to deal with it (that is, so they only ever see LF)?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

What's the proper content model for something like <a title="&#13;">? That is, should we be converting newline entities as well?

Converting newlines in the scanner does sound like the right thing to do. I think we have this problem with PIs, doctype decls, and some other tokens as well.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> That is, should we be converting newline entities as well?

See bug 197075.  So "probably yes".

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Sounds like a plan, then.

Comment 6

[Henri Sivonen (:hsivonen)]

See bug 557197 about 
.