2010-09-10 Adam Barth <abarth@webkit.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Sep 2010 21:25:13 +0000 (21:25 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Sep 2010 21:25:13 +0000 (21:25 +0000)
        Reviewed by Darin Adler.

        REGRESSION (HTML Parser): CNN's Money site is not formatted correctly with AdBlock installed
        https://bugs.webkit.org/show_bug.cgi?id=41371

        Drop support for closing comments with "-- >".  This was an attempt to
        be more IE-like in comment parsing, but it turns out to cause problems
        for some high-profile sites.  Firefox 4 is dropping support as well.
        We expect the spec to change soon too.

        * html/parser/HTMLTokenizer.cpp:
        (WebCore::HTMLTokenizer::nextToken):
        * html/parser/HTMLTokenizer.h:
2010-09-10  Adam Barth  <abarth@webkit.org>

        Reviewed by Darin Adler.

        REGRESSION (HTML Parser): CNN's Money site is not formatted correctly with AdBlock installed
        https://bugs.webkit.org/show_bug.cgi?id=41371

        Update test results for new behavior.

        * fast/parser/comments-expected.txt:
        * fast/parser/comments.html:
          - Removed the redundant in-test expectations.
        * html5lib/runner-expected.txt:
          - We now "fail" these two tests.  We'll pass them again once the spec
            changes and HTML5lib updates its expectations.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@67233 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/parser/comments-expected.txt
LayoutTests/fast/parser/comments.html
LayoutTests/html5lib/runner-expected.txt
WebCore/ChangeLog
WebCore/html/parser/HTMLTokenizer.cpp
WebCore/html/parser/HTMLTokenizer.h

index 8d1aa6a..8bd8610 100644 (file)
@@ -1,3 +1,19 @@
+2010-09-10  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        REGRESSION (HTML Parser): CNN's Money site is not formatted correctly with AdBlock installed
+        https://bugs.webkit.org/show_bug.cgi?id=41371
+
+        Update test results for new behavior.
+
+        * fast/parser/comments-expected.txt:
+        * fast/parser/comments.html:
+          - Removed the redundant in-test expectations.
+        * html5lib/runner-expected.txt:
+          - We now "fail" these two tests.  We'll pass them again once the spec
+            changes and HTML5lib updates its expectations.
+
 2010-09-10  Simon Fraser  <simon.fraser@apple.com>
 
         Reviewed by Adam Roben.
index 5e13471..9e5125e 100644 (file)
@@ -1,29 +1,22 @@
 Output of this test should match HTML5 (no strict SGML comment parsing).
 
-Basic comments (2 PASSED):
-PASSED PASSED
-Comment series (3 PASSED):
+Basic comments:
+PASSED
+Comment series:
+PASSED
+Dash runs:
 PASSED PASSED PASSED
 
-Dash runs (5 PASSED):
-PASSED PASSED PASSED PASSED PASSED
-
-Empty comments (5 PASSED):
-PASSED PASSED PASSED PASSED PASSED
-
-Multiple lines (4 PASSED):
-PASSED PASSED PASSED PASSED
-
-Compatibility (7 PASSED):
-PASSED PASSED PASSED PASSED PASSED PASSED PASSED
+Empty comments:
+PASSED PASSED PASSED
 
-White space after comment close (4 PASSED):
-PASSED PASSED PASSED PASSED
+Multiple lines:
+PASSED
 
-Text after comment close:
-Extra comment after markup declaration close (2 PASSED):
-FAILED: extra comment end and markup declaration close -->
+Compatibility:
 PASSED PASSED
 
-Nested comment (1 PASSED):
+White space after comment close:
+Extra comment after markup declaration close:
+FAILED: extra comment end and markup declaration close -->
 PASSED (outer nested comment) -->
index a23b686..17d9a34 100644 (file)
@@ -7,29 +7,29 @@
 if (window.layoutTestController)
     layoutTestController.dumpAsText();
 </script>
-<p>Basic comments (2 PASSED):<br>
+<p>Basic comments:<br>
 <!-- basic comment -->PASSED
 <!-- basic comment with spaces after comment end --    >PASSED
 <!-- basic comment with spaces after comment end - -    >FAILED--></p>
-<p>Comment series (3 PASSED):<br>
+<p>Comment series:<br>
 <!-- comment -- -- series-->PASSED
 <!-- comment -- -- series with space after comment end-- >PASSED
 <!-- comment -- -- series with spaces after comment end--  >PASSED</p>
 <!-- comment -- -- series with spaces after comment end- -  >FAILED--></p>
-<p>Dash runs (5 PASSED):<br>
+<p>Dash runs:<br>
 <!------ Hello -->PASSED
 <!------ Hello -- >PASSED
 <!------ Hello --  >PASSED
 <!------ Hello - -  >FAILED
 <!-- --- Hello -->PASSED
 <!-- Hello --->PASSED</p>
-<p>Empty comments (5 PASSED):<br>
+<p>Empty comments:<br>
 <!---->PASSED
 <!---- >PASSED
 <!>PASSED
 <!-->PASSED
 <!--->PASSED</p>
-<p>Multiple lines (4 PASSED):<br>
+<p>Multiple lines:<br>
 <!-- here's a comment, a little longer,
     which occupies more than one line -->PASSED
 <!-- here's a comment, a little longer,
@@ -43,7 +43,7 @@ if (window.layoutTestController)
     which occupies more than one line - -       >FAILED-->
 </p>
 
-<p>Compatibility (7 PASSED):<br>
+<p>Compatibility:<br>
 <!-- Compatibility: comment series with --extraneous-- text -- between -- the comments -->PASSED
 <!-- Compatibility: comment series with --extraneous-- text -- between -- the comments -- >PASSED
 <!-- Compatibility: comment series with --extraneous-- text -- between -- the comments --  >PASSED
@@ -55,7 +55,7 @@ if (window.layoutTestController)
 <!-- Compatibility: <!--extra comment start like www.the-leaky-cauldron.com has <rdar://problem/4226539> - -  >FAILED-->
 </p>
 
-<p>White space after comment close (4 PASSED):<br>
+<p>White space after comment close:<br>
 <!-- tab after comment close-- >PASSED
 <!-- LF after comment close--\f>PASSED
 <!-- CR after comment close--
@@ -67,12 +67,12 @@ if (window.layoutTestController)
 <!-- text after comment close--ouch>FAILED: should be part of the comment --></p>
 
 
-<p>Extra comment after markup declaration close (2 PASSED):<br>
+<p>Extra comment after markup declaration close:<br>
 <!-- Comment --> FAILED: extra comment end and markup declaration close --><br>
 <!-- Comment with a whitespace in markup declaration close -- >PASSED
 <!-- Comment with a whitespace in markup declaration close --    >PASSED</p>
 
-<p>Nested comment (1 PASSED):<br>
+<p>Nested comment:<br>
 <!-- nested: <!--FAILED (inner comment)--> PASSED (outer nested comment) --></p>
 
 </body>
index d12da74..974f6ca 100644 (file)
@@ -190,8 +190,41 @@ resources/entities01.dat: PASS
 
 resources/entities02.dat: PASS
 
-resources/comments01.dat: PASS
+resources/comments01.dat:
+3
+6
 
+Test 3 of 13 in resources/comments01.dat failed. Input:
+FOO<!-- BAR --   >BAZ
+Got:
+| <html>
+|   <head>
+|   <body>
+|     "FOO"
+|     <!--  BAR --   >BAZ -->
+Expected:
+| <html>
+|   <head>
+|   <body>
+|     "FOO"
+|     <!--  BAR --    -->
+|     "BAZ"
+
+Test 6 of 13 in resources/comments01.dat failed. Input:
+FOO<!-- BAR -- <QUX> -- MUX -- >BAZ
+Got:
+| <html>
+|   <head>
+|   <body>
+|     "FOO"
+|     <!--  BAR -- <QUX> -- MUX -- >BAZ -->
+Expected:
+| <html>
+|   <head>
+|   <body>
+|     "FOO"
+|     <!--  BAR -- <QUX> -- MUX --  -->
+|     "BAZ"
 resources/adoption01.dat: PASS
 
 resources/adoption02.dat:
index c58995c..faced09 100644 (file)
@@ -1,3 +1,19 @@
+2010-09-10  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        REGRESSION (HTML Parser): CNN's Money site is not formatted correctly with AdBlock installed
+        https://bugs.webkit.org/show_bug.cgi?id=41371
+
+        Drop support for closing comments with "-- >".  This was an attempt to
+        be more IE-like in comment parsing, but it turns out to cause problems
+        for some high-profile sites.  Firefox 4 is dropping support as well.
+        We expect the spec to change soon too.
+
+        * html/parser/HTMLTokenizer.cpp:
+        (WebCore::HTMLTokenizer::nextToken):
+        * html/parser/HTMLTokenizer.h:
+
 2010-09-10  Chris Marrin  <cmarrin@apple.com>
 
         Unreviewed.
index f5405ff..63612e6 100644 (file)
@@ -1213,13 +1213,7 @@ bool HTMLTokenizer::nextToken(SegmentedString& source, HTMLToken& token)
     BEGIN_STATE(CommentEndState) {
         if (cc == '>')
             return emitAndResumeIn(source, DataState);
-        else if (isTokenizerWhitespace(cc)) {
-            parseError();
-            m_token->appendToComment('-');
-            m_token->appendToComment('-');
-            m_token->appendToComment(cc);
-            ADVANCE_TO(CommentEndSpaceState);
-        } else if (cc == '!') {
+        else if (cc == '!') {
             parseError();
             ADVANCE_TO(CommentEndBangState);
         } else if (cc == '-') {
@@ -1260,24 +1254,6 @@ bool HTMLTokenizer::nextToken(SegmentedString& source, HTMLToken& token)
     }
     END_STATE()
 
-    BEGIN_STATE(CommentEndSpaceState) {
-        if (isTokenizerWhitespace(cc)) {
-            m_token->appendToComment(cc);
-            ADVANCE_TO(CommentEndSpaceState);
-        } else if (cc == '-')
-            ADVANCE_TO(CommentEndDashState);
-        else if (cc == '>')
-            return emitAndResumeIn(source, DataState);
-        else if (cc == InputStreamPreprocessor::endOfFileMarker) {
-            parseError();
-            return emitAndReconsumeIn(source, DataState);
-        } else {
-            m_token->appendToComment(cc);
-            ADVANCE_TO(CommentState);
-        }
-    }
-    END_STATE()
-
     BEGIN_STATE(DOCTYPEState) {
         if (isTokenizerWhitespace(cc))
             ADVANCE_TO(BeforeDOCTYPENameState);
index bab77f3..eb6eeda 100644 (file)
@@ -96,7 +96,6 @@ public:
         CommentEndDashState,
         CommentEndState,
         CommentEndBangState,
-        CommentEndSpaceState,
         DOCTYPEState,
         BeforeDOCTYPENameState,
         DOCTYPENameState,