<!DOCTYPE html><pre>&#x0a;&#x0a;A</pre> doesn't parse correctly
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Dec 2011 00:51:06 +0000 (00:51 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Dec 2011 00:51:06 +0000 (00:51 +0000)
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:

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

LayoutTests/ChangeLog
LayoutTests/html5lib/runner-expected.txt
LayoutTests/platform/chromium/html5lib/runner-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/html/parser/HTMLTokenizer.cpp
Source/WebCore/html/parser/HTMLTokenizer.h
Source/WebCore/html/parser/HTMLTreeBuilder.cpp
Source/WebCore/html/parser/HTMLTreeBuilder.h
Source/WebCore/xml/parser/MarkupTokenizerBase.h

index 3b59f8f..40bae38 100644 (file)
@@ -1,3 +1,14 @@
+2011-12-15  Adam Barth  <abarth@webkit.org>
+
+        <!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  <lmcliste@adobe.com>
 
         Create reftest for render-region-renderer
index 8f71264..e063f43 100644 (file)
Binary files a/LayoutTests/html5lib/runner-expected.txt and b/LayoutTests/html5lib/runner-expected.txt differ
index a5b01c0..3da11d3 100644 (file)
Binary files a/LayoutTests/platform/chromium/html5lib/runner-expected.txt and b/LayoutTests/platform/chromium/html5lib/runner-expected.txt differ
index 2aad4dc..6152d3f 100644 (file)
@@ -1,3 +1,32 @@
+2011-12-15  Adam Barth  <abarth@webkit.org>
+
+        <!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  <kbr@google.com>
 
         Unreviewed, rolling out r102989.
index 7a269be..9fb5ce3 100644 (file)
@@ -136,7 +136,6 @@ void HTMLTokenizer::reset()
     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 @@ bool HTMLTokenizer::nextToken(SegmentedString& source, HTMLToken& token)
         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) {
index 49af601..1e4798b 100644 (file)
@@ -146,10 +146,6 @@ public:
     //
     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; }
 
index dd968ca..90cb2b0 100644 (file)
@@ -268,6 +268,13 @@ public:
 
     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 @@ HTMLTreeBuilder::HTMLTreeBuilder(HTMLDocumentParser* parser, HTMLDocument* docum
     , 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 @@ HTMLTreeBuilder::HTMLTreeBuilder(HTMLDocumentParser* parser, DocumentFragment* f
     , 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 @@ void HTMLTreeBuilder::processStartTagForInBody(AtomicHTMLToken& token)
     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 @@ void HTMLTreeBuilder::processStartTagForInBody(AtomicHTMLToken& token)
     }
     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::processCharacter(AtomicHTMLToken& token)
 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);
index 32a19a7..bcc654a 100644 (file)
@@ -239,6 +239,8 @@ private:
     // 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;
index a1ca402..b8f9756 100644 (file)
@@ -197,7 +197,6 @@ protected:
     Token* m_token;
     int m_lineNumber;
 
-    bool m_skipLeadingNewLineForListing;
     bool m_forceNullCharacterReplacement;
 
     // http://www.whatwg.org/specs/web-apps/current-work/#additional-allowed-character