HTML parser fails to propertly close 4 identical nested formatting elements
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Sep 2012 22:40:15 +0000 (22:40 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Sep 2012 22:40:15 +0000 (22:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=96385

Reviewed by Adam Barth.

Add missing Adoption agency step 4.a to fix one of our two outlying Adoption Agency bugs.
This is the same step that Opera was missing (must have been recently added to the spec).

* html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::callTheAdoptionAgency):

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

LayoutTests/html5lib/runner-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/html/parser/HTMLTreeBuilder.cpp

index 3f68801..1c610d6 100644 (file)
@@ -9,7 +9,6 @@ CONSOLE MESSAGE: line 2: PASS
 CONSOLE MESSAGE: line 2: FOO<span>BAR</span>BAZ
 resources/adoption01.dat:
 14
-16
 
 Test 14 of 17 in resources/adoption01.dat failed. Input:
 <div><a><b><div><div><div><div><div><div><div><div><div><div></a>
@@ -65,29 +64,6 @@ Expected:
 |                         <a>
 |                         <div>
 |                           <div>
-
-Test 16 of 17 in resources/adoption01.dat failed. Input:
-<b><b><b><b>x</b></b></b></b>y
-Got:
-| <html>
-|   <head>
-|   <body>
-|     <b>
-|       <b>
-|         <b>
-|           <b>
-|             "x"
-|       "y"
-Expected:
-| <html>
-|   <head>
-|   <body>
-|     <b>
-|       <b>
-|         <b>
-|           <b>
-|             "x"
-|     "y"
 resources/adoption02.dat: PASS
 
 resources/comments01.dat: PASS
index c4c9a7b..733432a 100644 (file)
@@ -1,3 +1,16 @@
+2012-09-12  Eric Seidel  <eric@webkit.org>
+
+        HTML parser fails to propertly close 4 identical nested formatting elements
+        https://bugs.webkit.org/show_bug.cgi?id=96385
+
+        Reviewed by Adam Barth.
+
+        Add missing Adoption agency step 4.a to fix one of our two outlying Adoption Agency bugs.
+        This is the same step that Opera was missing (must have been recently added to the spec).
+
+        * html/parser/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
+
 2012-09-12  Siraj Razick  <siraj.razick@collabora.co.uk>
 
         [GTK] Protect RedirectedXCompositeWindow with USE(GLX) when building the clutter AC backend
index 522a440..3e9b303 100644 (file)
@@ -1420,74 +1420,81 @@ void HTMLTreeBuilder::callTheAdoptionAgency(AtomicHTMLToken* token)
     static const int outerIterationLimit = 8;
     static const int innerIterationLimit = 3;
 
+    // 1, 2, 3 and 16 are covered by the for() loop.
     for (int i = 0; i < outerIterationLimit; ++i) {
-        // 1.
+        // 4.
         Element* formattingElement = m_tree.activeFormattingElements()->closestElementInScopeWithName(token->name());
-        if (!formattingElement || ((m_tree.openElements()->contains(formattingElement)) && !m_tree.openElements()->inScope(formattingElement))) {
+        // 4.a
+        if (!formattingElement)
+            return processAnyOtherEndTagForInBody(token);
+        // 4.c
+        if ((m_tree.openElements()->contains(formattingElement)) && !m_tree.openElements()->inScope(formattingElement)) {
             parseError(token);
             notImplemented(); // Check the stack of open elements for a more specific parse error.
             return;
         }
+        // 4.b
         HTMLElementStack::ElementRecord* formattingElementRecord = m_tree.openElements()->find(formattingElement);
         if (!formattingElementRecord) {
             parseError(token);
             m_tree.activeFormattingElements()->remove(formattingElement);
             return;
         }
+        // 4.d
         if (formattingElement != m_tree.currentElement())
             parseError(token);
-        // 2.
+        // 5.
         HTMLElementStack::ElementRecord* furthestBlock = m_tree.openElements()->furthestBlockForFormattingElement(formattingElement);
-        // 3.
+        // 6.
         if (!furthestBlock) {
             m_tree.openElements()->popUntilPopped(formattingElement);
             m_tree.activeFormattingElements()->remove(formattingElement);
             return;
         }
-        // 4.
+        // 7.
         ASSERT(furthestBlock->isAbove(formattingElementRecord));
         RefPtr<HTMLStackItem> commonAncestor = formattingElementRecord->next()->stackItem();
-        // 5.
+        // 8.
         HTMLFormattingElementList::Bookmark bookmark = m_tree.activeFormattingElements()->bookmarkFor(formattingElement);
-        // 6.
+        // 9.
         HTMLElementStack::ElementRecord* node = furthestBlock;
         HTMLElementStack::ElementRecord* nextNode = node->next();
         HTMLElementStack::ElementRecord* lastNode = furthestBlock;
+        // 9.1, 9.2, 9.3 and 9.11 are covered by the for() loop.
         for (int i = 0; i < innerIterationLimit; ++i) {
-            // 6.1
+            // 9.4
             node = nextNode;
             ASSERT(node);
-            nextNode = node->next(); // Save node->next() for the next iteration in case node is deleted in 6.2.
-            // 6.2
+            nextNode = node->next(); // Save node->next() for the next iteration in case node is deleted in 9.5.
+            // 9.5
             if (!m_tree.activeFormattingElements()->contains(node->element())) {
                 m_tree.openElements()->remove(node->element());
                 node = 0;
                 continue;
             }
-            // 6.3
+            // 9.6
             if (node == formattingElementRecord)
                 break;
-            // 6.5
+            // 9.7
             RefPtr<HTMLStackItem> newItem = m_tree.createElementFromSavedToken(node->stackItem().get());
 
             HTMLFormattingElementList::Entry* nodeEntry = m_tree.activeFormattingElements()->find(node->element());
             nodeEntry->replaceElement(newItem);
             node->replaceElement(newItem.release());
-            // 6.4 -- Intentionally out of order to handle the case where node
-            // was replaced in 6.5.
-            // http://www.w3.org/Bugs/Public/show_bug.cgi?id=10096
+
+            // 9.8
             if (lastNode == furthestBlock)
                 bookmark.moveToAfter(nodeEntry);
-            // 6.6
+            // 9.9
             if (ContainerNode* parent = lastNode->element()->parentNode())
                 parent->parserRemoveChild(lastNode->element());
             node->element()->parserAddChild(lastNode->element());
             if (lastNode->element()->parentElement()->attached() && !lastNode->element()->attached())
                 lastNode->element()->lazyAttach();
-            // 6.7
+            // 9.10
             lastNode = node;
         }
-        // 7
+        // 10.
         if (ContainerNode* parent = lastNode->element()->parentNode())
             parent->parserRemoveChild(lastNode->element());
         if (commonAncestor->causesFosterParenting())
@@ -1499,24 +1506,25 @@ void HTMLTreeBuilder::callTheAdoptionAgency(AtomicHTMLToken* token)
             if (lastNode->element()->parentNode()->attached() && !lastNode->element()->attached())
                 lastNode->element()->lazyAttach();
         }
-        // 8
+        // 11.
         RefPtr<HTMLStackItem> newItem = m_tree.createElementFromSavedToken(formattingElementRecord->stackItem().get());
-        // 9
+        // 12.
         newItem->element()->takeAllChildrenFrom(furthestBlock->element());
-        // 10
+        // 13.
         Element* furthestBlockElement = furthestBlock->element();
         // FIXME: All this creation / parserAddChild / attach business should
-        //        be in HTMLConstructionSite.  My guess is that steps 8--12
+        //        be in HTMLConstructionSite. My guess is that steps 11--15
         //        should all be in some HTMLConstructionSite function.
         furthestBlockElement->parserAddChild(newItem->element());
+        // FIXME: Why is this attach logic necessary? Style resolve should attach us if needed.
         if (furthestBlockElement->attached() && !newItem->element()->attached()) {
             // Notice that newItem->element() might already be attached if, for example, one of the reparented
             // children is a style element, which attaches itself automatically.
             newItem->element()->attach();
         }
-        // 11
+        // 14.
         m_tree.activeFormattingElements()->swapTo(formattingElement, newItem, bookmark);
-        // 12
+        // 15.
         m_tree.openElements()->remove(formattingElement);
         m_tree.openElements()->insertAbove(newItem, furthestBlock);
     }