2010-08-11 Adam Barth <abarth@webkit.org>

        Reviewed by Eric Seidel.

        HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and colgroup
        https://bugs.webkit.org/show_bug.cgi?id=43758

        * html5lib/runner-expected-html5.txt:
2010-08-11  Adam Barth  <abarth@webkit.org>

        Reviewed by Eric Seidel.

        HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and colgroup
        https://bugs.webkit.org/show_bug.cgi?id=43758

        This patch conditionalizes some LegacyHTMLTreeBuilder-specific code in
        HTMLElement::createContextualFragment that interferes with the new
        HTMLTreeBuilder.  Doing that exposes the above ASSERT, which I've fixed
        in this patch too.  Fixing that ASSERT involved a small refactoring in
        ExternalCharacterTokenBuffer.

        * html/HTMLElement.cpp:
        (WebCore::HTMLElement::createContextualFragment):
        * html/HTMLTreeBuilder.cpp:
        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::skipLeadingWhitespace):
        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::takeLeadingWhitespace):
        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::takeLeadingNonWhitespace):
        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::skipLeading):
        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::takeLeading):
        (WebCore::HTMLTreeBuilder::processCharacterBuffer):

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 3470108..33303ed 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -2,6 +2,15 @@
 
         Reviewed by Eric Seidel.
 
+        HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and colgroup
+        https://bugs.webkit.org/show_bug.cgi?id=43758
+
+        * html5lib/runner-expected-html5.txt:
+
+2010-08-11  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
         Conditionalize wrong fragment parsing code to pass more HTML5lib tests
         https://bugs.webkit.org/show_bug.cgi?id=43877
 

diff --git a/LayoutTests/html5lib/runner-expected-html5.txt b/LayoutTests/html5lib/runner-expected-html5.txt
index fdbd8da..2eb01b4 100644 (file)
--- a/LayoutTests/html5lib/runner-expected-html5.txt
+++ b/LayoutTests/html5lib/runner-expected-html5.txt
@@ -16,35 +16,12 @@ resources/tests2.dat: PASS
 
 resources/tests3.dat: PASS
 
-resources/tests4.dat:
-7
+resources/tests4.dat: PASS
 
-Test 7 of 7 in resources/tests4.dat failed. Input:
-<title>setting head's innerHTML</title>
-Got:
-| 
-Expected:
-| <title>
-|   "setting head's innerHTML"
 resources/tests5.dat: PASS
 
-resources/tests6.dat:
-27
-30
+resources/tests6.dat: PASS
 
-Test 27 of 51 in resources/tests6.dat failed. Input:
-foo<col>
-Got:
-| 
-Expected:
-| <col>
-
-Test 30 of 51 in resources/tests6.dat failed. Input:
-</frameset><frame>
-Got:
-| 
-Expected:
-| <frame>
 resources/tests7.dat:
 30
 

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 6bc79a5..6585d0a 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -2,6 +2,29 @@
 
         Reviewed by Eric Seidel.
 
+        HTML TreeBuilder hits ASSERT in fragment case with insertAdjacentHTML and colgroup
+        https://bugs.webkit.org/show_bug.cgi?id=43758
+
+        This patch conditionalizes some LegacyHTMLTreeBuilder-specific code in
+        HTMLElement::createContextualFragment that interferes with the new
+        HTMLTreeBuilder.  Doing that exposes the above ASSERT, which I've fixed
+        in this patch too.  Fixing that ASSERT involved a small refactoring in
+        ExternalCharacterTokenBuffer.
+
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::createContextualFragment):
+        * html/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::skipLeadingWhitespace):
+        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::takeLeadingWhitespace):
+        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::takeLeadingNonWhitespace):
+        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::skipLeading):
+        (WebCore::HTMLTreeBuilder::ExternalCharacterTokenBuffer::takeLeading):
+        (WebCore::HTMLTreeBuilder::processCharacterBuffer):
+
+2010-08-11  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
         Conditionalize wrong fragment parsing code to pass more HTML5lib tests
         https://bugs.webkit.org/show_bug.cgi?id=43877
 

diff --git a/WebCore/html/HTMLElement.cpp b/WebCore/html/HTMLElement.cpp
index 3c3353c..b7d45de 100644 (file)
--- a/WebCore/html/HTMLElement.cpp
+++ b/WebCore/html/HTMLElement.cpp
@@ -275,16 +275,18 @@ String HTMLElement::outerHTML() const
     return createMarkup(this);
 }
 
-// FIXME: This method is unecessary with the new HTMLDocumentParser.
+// FIXME: This method is unnecessary with the new HTMLDocumentParser.
 PassRefPtr<DocumentFragment> HTMLElement::createContextualFragment(const String& markup, FragmentScriptingPermission scriptingPermission)
 {
-    // The following is in accordance with the definition as used by IE.
-    if (endTagRequirement() == TagStatusForbidden)
-        return 0;
+    if (!document()->settings() || !document()->settings()->html5TreeBuilderEnabled()) {
+        // The following is in accordance with the definition as used by IE.
+        if (endTagRequirement() == TagStatusForbidden)
+            return 0;
 
-    if (hasLocalName(colTag) || hasLocalName(colgroupTag) || hasLocalName(framesetTag) ||
-        hasLocalName(headTag) || hasLocalName(styleTag) || hasLocalName(titleTag))
-        return 0;
+        if (hasLocalName(colTag) || hasLocalName(colgroupTag) || hasLocalName(framesetTag)
+            || hasLocalName(headTag) || hasLocalName(styleTag) || hasLocalName(titleTag))
+            return 0;
+    }
 
     return Element::createContextualFragment(markup, scriptingPermission);
 }

diff --git a/WebCore/html/HTMLTreeBuilder.cpp b/WebCore/html/HTMLTreeBuilder.cpp
index ab7e6bf..1d23539 100644 (file)
--- a/WebCore/html/HTMLTreeBuilder.cpp
+++ b/WebCore/html/HTMLTreeBuilder.cpp
@@ -66,6 +66,11 @@ inline bool isTreeBuilderWhitepace(UChar c)
     return c == '\t' || c == '\x0A' || c == '\x0C' || c == '\x0D' || c == ' ';
 }
 
+inline bool isNotTreeBuilderWhitepace(UChar c)
+{
+    return !isTreeBuilderWhitepace(c);
+}
+
 inline bool isTreeBuilderWhitepaceOrReplacementCharacter(UChar c)
 {
     return isTreeBuilderWhitepace(c) || c == 0xFFFD;
@@ -302,21 +307,17 @@ public:
 
     void skipLeadingWhitespace()
     {
-        ASSERT(!isEmpty());
-        while (isTreeBuilderWhitepace(*m_current)) {
-            if (++m_current == m_end)
-                return;
-        }
+        skipLeading<isTreeBuilderWhitepace>();
     }
 
     String takeLeadingWhitespace()
     {
-        ASSERT(!isEmpty());
-        const UChar* start = m_current;
-        skipLeadingWhitespace();
-        if (start == m_current)
-            return String();
-        return String(start, m_current - start);
+        return takeLeading<isTreeBuilderWhitepace>();
+    }
+
+    String takeLeadingNonWhitespace()
+    {
+        return takeLeading<isNotTreeBuilderWhitepace>();
     }
 
     String takeRemaining()
@@ -351,6 +352,27 @@ public:
     }
 
 private:
+    template<bool characterPredicate(UChar)>
+    void skipLeading()
+    {
+        ASSERT(!isEmpty());
+        while (characterPredicate(*m_current)) {
+            if (++m_current == m_end)
+                return;
+        }
+    }
+
+    template<bool characterPredicate(UChar)>
+    String takeLeading()
+    {
+        ASSERT(!isEmpty());
+        const UChar* start = m_current;
+        skipLeading<characterPredicate>();
+        if (start == m_current)
+            return String();
+        return String(start, m_current - start);
+    }
+
     const UChar* m_current;
     const UChar* m_end;
 };
@@ -2629,7 +2651,10 @@ ReprocessBuffer:
             return;
         if (!processColgroupEndTagForInColumnGroup()) {
             ASSERT(isParsingFragment());
-            return;
+            // The spec tells us to drop these characters on the floor.
+            buffer.takeLeadingNonWhitespace();
+            if (buffer.isEmpty())
+                return;
         }
         goto ReprocessBuffer;
     }