[webkit-changes] [103000] trunk

Thu, 15 Dec 2011 16:51:41 -0800

Title: [103000] trunk
Revision
103000
Author
[email protected]
Date
2011-12-15 16:51:06 -0800 (Thu, 15 Dec 2011)

Log Message

<!DOCTYPE html><pre>&#x0a;&#x0a;A</pre> doesn't parse correctly
https://bugs.webkit.org/show_bug.cgi?id=74658

Reviewed by Darin Adler.

Source/WebCore: 

Previously, we handled skipping newlines after <pre> in the tokenizer,
which isn't how the spec handles them.  Instead, the spec skips them in
the tree builder.  This isn't usually observable, except in the case of
an HTML entity.  In that case, the tokenzier sees '&' (because the
entity hasn't been decoded yet), but the tree builder sees '\n' (the
decoded entity).  This patch fixes the bug by more closely aligning our
implementation with the spec.

Test: html5lib/runner.html

* html/parser/HTMLTokenizer.cpp:
(WebCore::HTMLTokenizer::reset):
(WebCore::HTMLTokenizer::nextToken):
* html/parser/HTMLTokenizer.h:
* html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::skipAtMostOneLeadingNewline):
(WebCore::HTMLTreeBuilder::HTMLTreeBuilder):
(WebCore::HTMLTreeBuilder::processStartTagForInBody):
(WebCore::HTMLTreeBuilder::processCharacterBuffer):
* html/parser/HTMLTreeBuilder.h:
* xml/parser/MarkupTokenizerBase.h:

LayoutTests: 

Shows test progression.

* html5lib/runner-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (102999 => 103000)


--- trunk/LayoutTests/ChangeLog	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/LayoutTests/ChangeLog	2011-12-16 00:51:06 UTC (rev 103000)
@@ -1,3 +1,14 @@
+2011-12-15  Adam Barth  <[email protected]>
+
+        <!DOCTYPE html><pre>&#x0a;&#x0a;A</pre> doesn't parse correctly
+        https://bugs.webkit.org/show_bug.cgi?id=74658
+
+        Reviewed by Darin Adler.
+
+        Shows test progression.
+
+        * html5lib/runner-expected.txt:
+
 2011-12-15  Larry McLister  <[email protected]>
 
         Create reftest for render-region-renderer

Modified: trunk/LayoutTests/html5lib/runner-expected.txt (102999 => 103000)


--- trunk/LayoutTests/html5lib/runner-expected.txt	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/LayoutTests/html5lib/runner-expected.txt	2011-12-16 00:51:06 UTC (rev 103000)
@@ -126,28 +126,8 @@
 
 resources/tests2.dat: PASS
 
-resources/tests3.dat:
-12
+resources/tests3.dat: PASS
 
-Test 12 of 24 in resources/tests3.dat failed. Input:
-<!DOCTYPE html><pre>&#x0a;&#x0a;A</pre>
-Got:
-| <!DOCTYPE html>
-| <html>
-|   <head>
-|   <body>
-|     <pre>
-|       "
-
-A"
-Expected:
-| <!DOCTYPE html>
-| <html>
-|   <head>
-|   <body>
-|     <pre>
-|       "
-A"
 resources/tests4.dat: PASS
 
 resources/tests5.dat: PASS

Modified: trunk/LayoutTests/platform/chromium/html5lib/runner-expected.txt (102999 => 103000)


--- trunk/LayoutTests/platform/chromium/html5lib/runner-expected.txt	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/LayoutTests/platform/chromium/html5lib/runner-expected.txt	2011-12-16 00:51:06 UTC (rev 103000)
@@ -126,28 +126,8 @@
 
 resources/tests2.dat: PASS
 
-resources/tests3.dat:
-12
+resources/tests3.dat: PASS
 
-Test 12 of 24 in resources/tests3.dat failed. Input:
-<!DOCTYPE html><pre>&#x0a;&#x0a;A</pre>
-Got:
-| <!DOCTYPE html>
-| <html>
-|   <head>
-|   <body>
-|     <pre>
-|       "
-
-A"
-Expected:
-| <!DOCTYPE html>
-| <html>
-|   <head>
-|   <body>
-|     <pre>
-|       "
-A"
 resources/tests4.dat: PASS
 
 resources/tests5.dat: PASS

Modified: trunk/Source/WebCore/ChangeLog (102999 => 103000)


--- trunk/Source/WebCore/ChangeLog	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/Source/WebCore/ChangeLog	2011-12-16 00:51:06 UTC (rev 103000)
@@ -1,3 +1,32 @@
+2011-12-15  Adam Barth  <[email protected]>
+
+        <!DOCTYPE html><pre>&#x0a;&#x0a;A</pre> doesn't parse correctly
+        https://bugs.webkit.org/show_bug.cgi?id=74658
+
+        Reviewed by Darin Adler.
+
+        Previously, we handled skipping newlines after <pre> in the tokenizer,
+        which isn't how the spec handles them.  Instead, the spec skips them in
+        the tree builder.  This isn't usually observable, except in the case of
+        an HTML entity.  In that case, the tokenzier sees '&' (because the
+        entity hasn't been decoded yet), but the tree builder sees '\n' (the
+        decoded entity).  This patch fixes the bug by more closely aligning our
+        implementation with the spec.
+
+        Test: html5lib/runner.html
+
+        * html/parser/HTMLTokenizer.cpp:
+        (WebCore::HTMLTokenizer::reset):
+        (WebCore::HTMLTokenizer::nextToken):
+        * html/parser/HTMLTokenizer.h:
+        * html/parser/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::skipAtMostOneLeadingNewline):
+        (WebCore::HTMLTreeBuilder::HTMLTreeBuilder):
+        (WebCore::HTMLTreeBuilder::processStartTagForInBody):
+        (WebCore::HTMLTreeBuilder::processCharacterBuffer):
+        * html/parser/HTMLTreeBuilder.h:
+        * xml/parser/MarkupTokenizerBase.h:
+
 2011-12-15  Kenneth Russell  <[email protected]>
 
         Unreviewed, rolling out r102989.

Modified: trunk/Source/WebCore/html/parser/HTMLTokenizer.cpp (102999 => 103000)


--- trunk/Source/WebCore/html/parser/HTMLTokenizer.cpp	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/Source/WebCore/html/parser/HTMLTokenizer.cpp	2011-12-16 00:51:06 UTC (rev 103000)
@@ -136,7 +136,6 @@
     m_state = HTMLTokenizerState::DataState;
     m_token = 0;
     m_lineNumber = 0;
-    m_skipLeadingNewLineForListing = false;
     m_forceNullCharacterReplacement = false;
     m_shouldAllowCDATA = false;
     m_additionalAllowedCharacter = '\0';
@@ -211,30 +210,6 @@
         return haveBufferedCharacterToken();
     UChar cc = m_inputStreamPreprocessor.nextInputCharacter();
 
-    // http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#parsing-main-inbody
-    // Note that this logic is different than the generic \r\n collapsing
-    // handled in the input stream preprocessor. This logic is here as an
-    // "authoring convenience" so folks can write:
-    //
-    // <pre>
-    // lorem ipsum
-    // lorem ipsum
-    // </pre>
-    //
-    // without getting an extra newline at the start of their <pre> element.
-    if (m_skipLeadingNewLineForListing) {
-        m_skipLeadingNewLineForListing = false;
-        if (cc == '\n') {
-            if (m_state == HTMLTokenizerState::DataState)
-                HTML_ADVANCE_TO(DataState);
-            if (m_state == HTMLTokenizerState::RCDATAState)
-                HTML_ADVANCE_TO(RCDATAState);
-            // When parsing text/plain documents, we run the tokenizer in the
-            // PLAINTEXTState and ignore m_skipLeadingNewLineForListing.
-            ASSERT(m_state == HTMLTokenizerState::PLAINTEXTState);
-        }
-    }
-
     // Source: http://www.whatwg.org/specs/web-apps/current-work/#tokenisation0
     switch (m_state) {
     HTML_BEGIN_STATE(DataState) {

Modified: trunk/Source/WebCore/html/parser/HTMLTokenizer.h (102999 => 103000)


--- trunk/Source/WebCore/html/parser/HTMLTokenizer.h	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/Source/WebCore/html/parser/HTMLTokenizer.h	2011-12-16 00:51:06 UTC (rev 103000)
@@ -146,10 +146,6 @@
     //
     void updateStateFor(const AtomicString& tagName, Frame*);
 
-    // Hack to skip leading newline in <pre>/<listing> for authoring ease.
-    // http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#parsing-main-inbody
-    void setSkipLeadingNewLineForListing(bool value) { m_skipLeadingNewLineForListing = value; }
-
     bool forceNullCharacterReplacement() const { return m_forceNullCharacterReplacement; }
     void setForceNullCharacterReplacement(bool value) { m_forceNullCharacterReplacement = value; }

Modified: trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp (102999 => 103000)


--- trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/Source/WebCore/html/parser/HTMLTreeBuilder.cpp	2011-12-16 00:51:06 UTC (rev 103000)
@@ -268,6 +268,13 @@
 
     bool isEmpty() const { return m_current == m_end; }
 
+    void skipAtMostOneLeadingNewline()
+    {
+        ASSERT(!isEmpty());
+        if (*m_current == '\n')
+            ++m_current;
+    }
+
     void skipLeadingWhitespace()
     {
         skipLeading<isHTMLSpace>();
@@ -349,6 +356,7 @@
     , m_isPaused(false)
     , m_insertionMode(InitialMode)
     , m_originalInsertionMode(InitialMode)
+    , m_shouldSkipLeadingNewline(false)
     , m_parser(parser)
     , m_scriptToProcessStartPosition(uninitializedPositionValue1())
     , m_lastScriptElementStartPosition(TextPosition::belowRangePosition())
@@ -367,6 +375,7 @@
     , m_isPaused(false)
     , m_insertionMode(InitialMode)
     , m_originalInsertionMode(InitialMode)
+    , m_shouldSkipLeadingNewline(false)
     , m_parser(parser)
     , m_scriptToProcessStartPosition(uninitializedPositionValue1())
     , m_lastScriptElementStartPosition(TextPosition::belowRangePosition())
@@ -810,7 +819,7 @@
     if (token.name() == preTag || token.name() == listingTag) {
         processFakePEndTagIfPInButtonScope();
         m_tree.insertHTMLElement(token);
-        m_parser->tokenizer()->setSkipLeadingNewLineForListing(true);
+        m_shouldSkipLeadingNewline = true;
         m_framesetOk = false;
         return;
     }
@@ -937,7 +946,7 @@
     }
     if (token.name() == textareaTag) {
         m_tree.insertHTMLElement(token);
-        m_parser->tokenizer()->setSkipLeadingNewLineForListing(true);
+        m_shouldSkipLeadingNewline = true;
         m_parser->tokenizer()->setState(HTMLTokenizerState::RCDATAState);
         m_originalInsertionMode = m_insertionMode;
         m_framesetOk = false;
@@ -2274,6 +2283,24 @@
 void HTMLTreeBuilder::processCharacterBuffer(ExternalCharacterTokenBuffer& buffer)
 {
 ReprocessBuffer:
+    // http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#parsing-main-inbody
+    // Note that this logic is different than the generic \r\n collapsing
+    // handled in the input stream preprocessor. This logic is here as an
+    // "authoring convenience" so folks can write:
+    //
+    // <pre>
+    // lorem ipsum
+    // lorem ipsum
+    // </pre>
+    //
+    // without getting an extra newline at the start of their <pre> element.
+    if (m_shouldSkipLeadingNewline) {
+        m_shouldSkipLeadingNewline = false;
+        buffer.skipAtMostOneLeadingNewline();
+        if (buffer.isEmpty())
+            return;
+    }
+
     switch (insertionMode()) {
     case InitialMode: {
         ASSERT(insertionMode() == InitialMode);

Modified: trunk/Source/WebCore/html/parser/HTMLTreeBuilder.h (102999 => 103000)


--- trunk/Source/WebCore/html/parser/HTMLTreeBuilder.h	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/Source/WebCore/html/parser/HTMLTreeBuilder.h	2011-12-16 00:51:06 UTC (rev 103000)
@@ -239,6 +239,8 @@
     // http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#pending-table-character-tokens
     StringBuilder m_pendingTableCharacters;
 
+    bool m_shouldSkipLeadingNewline;
+
     // We access parser because HTML5 spec requires that we be able to change the state of the tokenizer
     // from within parser actions. We also need it to track the current position.
     HTMLDocumentParser* m_parser;

Modified: trunk/Source/WebCore/xml/parser/MarkupTokenizerBase.h (102999 => 103000)


--- trunk/Source/WebCore/xml/parser/MarkupTokenizerBase.h	2011-12-16 00:44:56 UTC (rev 102999)
+++ trunk/Source/WebCore/xml/parser/MarkupTokenizerBase.h	2011-12-16 00:51:06 UTC (rev 103000)
@@ -197,7 +197,6 @@
     Token* m_token;
     int m_lineNumber;
 
-    bool m_skipLeadingNewLineForListing;
     bool m_forceNullCharacterReplacement;
 
     // http://www.whatwg.org/specs/web-apps/current-work/#additional-allowed-character

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to