[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

eric at webkit.org eric at webkit.org
Wed Dec 22 11:11:06 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit f7a765313ff37bc894cbc96028c5d191241e9b8f
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Jul 14 18:49:42 2010 +0000

    2010-07-13  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            reconstructActiveFormElements should reconstruct attributes as well
            https://bugs.webkit.org/show_bug.cgi?id=42222
    
            * html5lib/resources/adoption01.dat:
            * html5lib/runner-expected-html5.txt:
            * html5lib/runner-expected.txt:
    2010-07-13  Eric Seidel  <eric at webkit.org>
    
            Reviewed by Adam Barth.
    
            reconstructActiveFormElements should reconstruct attributes as well
            https://bugs.webkit.org/show_bug.cgi?id=42222
    
            The case in question is "<p><b foo='bar'></p>text</b>".
            When the "b" is re-opened to wrap the text it should include
            any attributes from the original (now closed) tag name.
    
            There are also similar cases for the Adoption Agency algorithm, but since
            the html5lib test suite did not cover those (and it wasn't immediately
            obvious to me how to test those) I've saved fixing that bug for a
            later patch.  For now I've just made the adoption agency use
            HTMLConstructionSite::createHTMLElementFromElementRecord so the
            FIXME can be in one place instead of two.
    
            In order to cleanly support createHTMLElementFromSavedElement
            I re-factored "attachToCurrent" out from createHTMLElementAndAttachToCurrent
            and changed all callers to use attachToCurrent(createHTMLElement(token)).
    
            This is covered by two existing tests in html5lib/runner.html
            and I wrote two more.  One to cover the basic case that we now pass
            and a second to cover an evil edge case which we do not.
    
            * html/HTMLConstructionSite.cpp:
            (WebCore::HTMLConstructionSite::attachToCurrent):
            (WebCore::HTMLConstructionSite::insertHTMLHtmlElement):
            (WebCore::HTMLConstructionSite::insertHTMLHeadElement):
            (WebCore::HTMLConstructionSite::insertHTMLBodyElement):
            (WebCore::HTMLConstructionSite::insertHTMLElement):
            (WebCore::HTMLConstructionSite::insertSelfClosingHTMLElement):
            (WebCore::HTMLConstructionSite::insertScriptElement):
            (WebCore::HTMLConstructionSite::insertForeignElement):
            (WebCore::HTMLConstructionSite::createHTMLElementFromElementRecord):
            (WebCore::HTMLConstructionSite::createHTMLElementFromSavedElement):
            (WebCore::HTMLConstructionSite::reconstructTheActiveFormattingElements):
            * html/HTMLConstructionSite.h:
            * html/HTMLTreeBuilder.cpp:
            (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@63338 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 20efa24..6956b34 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2010-07-13  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
+        reconstructActiveFormElements should reconstruct attributes as well
+        https://bugs.webkit.org/show_bug.cgi?id=42222
+
+        * html5lib/resources/adoption01.dat:
+        * html5lib/runner-expected-html5.txt:
+        * html5lib/runner-expected.txt:
+
 2010-07-14  Kenneth Russell  <kbr at google.com>
 
         Reviewed by Darin Fisher.
diff --git a/LayoutTests/html5lib/resources/adoption01.dat b/LayoutTests/html5lib/resources/adoption01.dat
index f80c4c6..01e7948 100644
--- a/LayoutTests/html5lib/resources/adoption01.dat
+++ b/LayoutTests/html5lib/resources/adoption01.dat
@@ -124,3 +124,43 @@
 |       <b>
 |         <p>
 |           <a>
+
+#data
+<p>1<s id="A">2<b id="B">3</p>4</s>5</b>
+#errors
+#document
+| <html>
+|   <head>
+|   <body>
+|     <p>
+|       "1"
+|       <s>
+|         id="A"
+|         "2"
+|         <b>
+|           id="B"
+|           "3"
+|     <s>
+|       id="A"
+|       <b>
+|         id="B"
+|         "4"
+|     <b>
+|       id="B"
+|       "5"
+
+#data
+<p><b id="A"><script>document.getElementById("A").id = "B"</script></p>TEXT</b>
+#errors
+#document
+| <html>
+|   <head>
+|   <body>
+|     <p>
+|       <b>
+|         id="B"
+|         <script>
+|           "document.getElementById("A").id = "B""
+|     <b>
+|       id="A"
+|       "TEXT"
diff --git a/LayoutTests/html5lib/runner-expected-html5.txt b/LayoutTests/html5lib/runner-expected-html5.txt
index 4ede9cd..fd6eab2 100644
--- a/LayoutTests/html5lib/runner-expected-html5.txt
+++ b/LayoutTests/html5lib/runner-expected-html5.txt
@@ -23,12 +23,14 @@ Got:
 |         href="foo"
 |         "br"
 |       <a>
+|         href="foo"
 |       "x"
 |       <table>
 |         <tbody>
 |           <tr>
 |             <td>
 |     <a>
+|       href="foo"
 |       "aoe"
 Expected:
 | <html>
@@ -61,6 +63,7 @@ Got:
 |       href="blah"
 |       "aba"
 |     <a>
+|       href="blah"
 |     "x"
 |     <table>
 |       <tbody>
@@ -70,6 +73,7 @@ Got:
 |               href="foo"
 |               "br"
 |     <a>
+|       href="blah"
 |       "aoe"
 Expected:
 | <html>
@@ -322,8 +326,9 @@ resources/comments01.dat: PASS
 
 resources/adoption01.dat:
 3
+11
 
-Test 3 of 9 in resources/adoption01.dat failed. Input:
+Test 3 of 11 in resources/adoption01.dat failed. Input:
 <a>1<button>2</a>3</button>
 Got:
 | <html>
@@ -342,6 +347,33 @@ Expected:
 |       <button>
 |         "2"
 |     "3"
+
+Test 11 of 11 in resources/adoption01.dat failed. Input:
+<p><b id="A"><script>document.getElementById("A").id = "B"</script></p>TEXT</b>
+Got:
+| <html>
+|   <head>
+|   <body>
+|     <p>
+|       <b>
+|         id="B"
+|         <script>
+|           "document.getElementById("A").id = "B""
+|     <b>
+|       id="B"
+|       "TEXT"
+Expected:
+| <html>
+|   <head>
+|   <body>
+|     <p>
+|       <b>
+|         id="B"
+|         <script>
+|           "document.getElementById("A").id = "B""
+|     <b>
+|       id="A"
+|       "TEXT"
 resources/inbody01.dat: PASS
 
 resources/isindex.dat: PASS
diff --git a/LayoutTests/html5lib/runner-expected.txt b/LayoutTests/html5lib/runner-expected.txt
index ce9899e..ff1d8ed 100644
--- a/LayoutTests/html5lib/runner-expected.txt
+++ b/LayoutTests/html5lib/runner-expected.txt
@@ -5003,8 +5003,9 @@ resources/adoption01.dat:
 7
 8
 9
+11
 
-Test 1 of 9 in resources/adoption01.dat failed. Input:
+Test 1 of 11 in resources/adoption01.dat failed. Input:
 <a><p></a></p>
 Got:
 | <html>
@@ -5020,7 +5021,7 @@ Expected:
 |     <p>
 |       <a>
 
-Test 6 of 9 in resources/adoption01.dat failed. Input:
+Test 6 of 11 in resources/adoption01.dat failed. Input:
 <table><a>1<p>2</a>3</p>
 Got:
 | <html>
@@ -5045,7 +5046,7 @@ Expected:
 |       "3"
 |     <table>
 
-Test 7 of 9 in resources/adoption01.dat failed. Input:
+Test 7 of 11 in resources/adoption01.dat failed. Input:
 <b><b><a><p></a>
 Got:
 | <html>
@@ -5065,7 +5066,7 @@ Expected:
 |         <p>
 |           <a>
 
-Test 8 of 9 in resources/adoption01.dat failed. Input:
+Test 8 of 11 in resources/adoption01.dat failed. Input:
 <b><a><b><p></a>
 Got:
 | <html>
@@ -5087,7 +5088,7 @@ Expected:
 |         <p>
 |           <a>
 
-Test 9 of 9 in resources/adoption01.dat failed. Input:
+Test 9 of 11 in resources/adoption01.dat failed. Input:
 <a><b><b><p></a>
 Got:
 | <html>
@@ -5110,6 +5111,33 @@ Expected:
 |       <b>
 |         <p>
 |           <a>
+
+Test 11 of 11 in resources/adoption01.dat failed. Input:
+<p><b id="A"><script>document.getElementById("A").id = "B"</script></p>TEXT</b>
+Got:
+| <html>
+|   <head>
+|   <body>
+|     <p>
+|       <b>
+|         id="B"
+|         <script>
+|           "document.getElementById("A").id = "B""
+|     <b>
+|       id="B"
+|       "TEXT"
+Expected:
+| <html>
+|   <head>
+|   <body>
+|     <p>
+|       <b>
+|         id="B"
+|         <script>
+|           "document.getElementById("A").id = "B""
+|     <b>
+|       id="A"
+|       "TEXT"
 resources/inbody01.dat: PASS
 
 resources/isindex.dat:
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 84de405..528522b 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,45 @@
+2010-07-13  Eric Seidel  <eric at webkit.org>
+
+        Reviewed by Adam Barth.
+
+        reconstructActiveFormElements should reconstruct attributes as well
+        https://bugs.webkit.org/show_bug.cgi?id=42222
+
+        The case in question is "<p><b foo='bar'></p>text</b>".
+        When the "b" is re-opened to wrap the text it should include
+        any attributes from the original (now closed) tag name.
+
+        There are also similar cases for the Adoption Agency algorithm, but since
+        the html5lib test suite did not cover those (and it wasn't immediately
+        obvious to me how to test those) I've saved fixing that bug for a
+        later patch.  For now I've just made the adoption agency use
+        HTMLConstructionSite::createHTMLElementFromElementRecord so the
+        FIXME can be in one place instead of two.
+
+        In order to cleanly support createHTMLElementFromSavedElement
+        I re-factored "attachToCurrent" out from createHTMLElementAndAttachToCurrent
+        and changed all callers to use attachToCurrent(createHTMLElement(token)).
+
+        This is covered by two existing tests in html5lib/runner.html
+        and I wrote two more.  One to cover the basic case that we now pass
+        and a second to cover an evil edge case which we do not.
+
+        * html/HTMLConstructionSite.cpp:
+        (WebCore::HTMLConstructionSite::attachToCurrent):
+        (WebCore::HTMLConstructionSite::insertHTMLHtmlElement):
+        (WebCore::HTMLConstructionSite::insertHTMLHeadElement):
+        (WebCore::HTMLConstructionSite::insertHTMLBodyElement):
+        (WebCore::HTMLConstructionSite::insertHTMLElement):
+        (WebCore::HTMLConstructionSite::insertSelfClosingHTMLElement):
+        (WebCore::HTMLConstructionSite::insertScriptElement):
+        (WebCore::HTMLConstructionSite::insertForeignElement):
+        (WebCore::HTMLConstructionSite::createHTMLElementFromElementRecord):
+        (WebCore::HTMLConstructionSite::createHTMLElementFromSavedElement):
+        (WebCore::HTMLConstructionSite::reconstructTheActiveFormattingElements):
+        * html/HTMLConstructionSite.h:
+        * html/HTMLTreeBuilder.cpp:
+        (WebCore::HTMLTreeBuilder::callTheAdoptionAgency):
+
 2010-07-13  Alexey Proskuryakov  <ap at apple.com>
 
         Reviewed by Darin Adler.
diff --git a/WebCore/html/HTMLConstructionSite.cpp b/WebCore/html/HTMLConstructionSite.cpp
index 5745625..1e2d75f 100644
--- a/WebCore/html/HTMLConstructionSite.cpp
+++ b/WebCore/html/HTMLConstructionSite.cpp
@@ -193,40 +193,39 @@ void HTMLConstructionSite::insertCommentOnHTMLHtmlElement(AtomicHTMLToken& token
     attach(m_openElements.htmlElement(), Comment::create(m_document, token.comment()));
 }
 
-PassRefPtr<Element> HTMLConstructionSite::createHTMLElementAndAttachToCurrent(AtomicHTMLToken& token)
+PassRefPtr<Element> HTMLConstructionSite::attachToCurrent(PassRefPtr<Element> child)
 {
-    ASSERT(token.type() == HTMLToken::StartTag);
-    return attach(currentElement(), createHTMLElement(token));
+    return attach(currentElement(), child);
 }
 
 void HTMLConstructionSite::insertHTMLHtmlElement(AtomicHTMLToken& token)
 {
     ASSERT(!m_redirectAttachToFosterParent);
-    m_openElements.pushHTMLHtmlElement(createHTMLElementAndAttachToCurrent(token));
+    m_openElements.pushHTMLHtmlElement(attachToCurrent(createHTMLElement(token)));
 }
 
 void HTMLConstructionSite::insertHTMLHeadElement(AtomicHTMLToken& token)
 {
     ASSERT(!m_redirectAttachToFosterParent);
-    m_head = createHTMLElementAndAttachToCurrent(token);
+    m_head = attachToCurrent(createHTMLElement(token));
     m_openElements.pushHTMLHeadElement(m_head);
 }
 
 void HTMLConstructionSite::insertHTMLBodyElement(AtomicHTMLToken& token)
 {
     ASSERT(!m_redirectAttachToFosterParent);
-    m_openElements.pushHTMLBodyElement(createHTMLElementAndAttachToCurrent(token));
+    m_openElements.pushHTMLBodyElement(attachToCurrent(createHTMLElement(token)));
 }
 
 void HTMLConstructionSite::insertHTMLElement(AtomicHTMLToken& token)
 {
-    m_openElements.push(createHTMLElementAndAttachToCurrent(token));
+    m_openElements.push(attachToCurrent(createHTMLElement(token)));
 }
 
 void HTMLConstructionSite::insertSelfClosingHTMLElement(AtomicHTMLToken& token)
 {
     ASSERT(token.type() == HTMLToken::StartTag);
-    createHTMLElementAndAttachToCurrent(token);
+    attachToCurrent(createHTMLElement(token));
     // FIXME: Do we want to acknowledge the token's self-closing flag?
     // http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#acknowledge-self-closing-flag
 }
@@ -244,7 +243,7 @@ void HTMLConstructionSite::insertScriptElement(AtomicHTMLToken& token)
 {
     RefPtr<HTMLScriptElement> element = HTMLScriptElement::create(scriptTag, m_document, true);
     element->setAttributeMap(token.takeAtributes(), m_fragmentScriptingPermission);
-    m_openElements.push(attach(currentElement(), element.release()));
+    m_openElements.push(attachToCurrent(element.release()));
 }
 
 void HTMLConstructionSite::insertForeignElement(AtomicHTMLToken& token, const AtomicString& namespaceURI)
@@ -252,7 +251,7 @@ void HTMLConstructionSite::insertForeignElement(AtomicHTMLToken& token, const At
     ASSERT(token.type() == HTMLToken::StartTag);
     notImplemented(); // parseError when xmlns or xmlns:xlink are wrong.
 
-    RefPtr<Element> element = attach(currentElement(), createElement(token, namespaceURI));
+    RefPtr<Element> element = attachToCurrent(createElement(token, namespaceURI));
     if (!token.selfClosing())
         m_openElements.push(element);
 }
@@ -292,6 +291,48 @@ PassRefPtr<Element> HTMLConstructionSite::createHTMLElement(AtomicHTMLToken& tok
     return element.release();
 }
 
+PassRefPtr<Element> HTMLConstructionSite::createHTMLElementFromElementRecord(HTMLElementStack::ElementRecord* record)
+{
+    // FIXME: This will change to use
+    // return createHTMLElementFromSavedElement(record->element());
+    // in a later patch once tested.
+    AtomicHTMLToken fakeToken(HTMLToken::StartTag, record->element()->localName());
+    return createHTMLElement(fakeToken);
+}
+
+namespace {
+
+PassRefPtr<NamedNodeMap> cloneAttributes(Element* element)
+{
+    NamedNodeMap* attributes = element->attributes(true);
+    if (!attributes)
+        return 0;
+
+    RefPtr<NamedNodeMap> newAttributes = NamedNodeMap::create();
+    for (size_t i = 0; i < attributes->length(); ++i) {
+        Attribute* attribute = attributes->attributeItem(i);
+        RefPtr<Attribute> clone = Attribute::createMapped(attribute->name(), attribute->value());
+        newAttributes->addAttribute(clone);
+    }
+    return newAttributes.release();
+}
+
+}
+
+PassRefPtr<Element> HTMLConstructionSite::createHTMLElementFromSavedElement(Element* element)
+{
+    // FIXME: This method is wrong.  We should be using the original token.
+    // Using an Element* causes us to fail examples like this:
+    // <b id="1"><p><script>document.getElementById("1").id = "2"</script></p>TEXT</b>
+    // When reconstructActiveFormattingElements calls this method to open
+    // a second <b> tag to wrap TEXT, it will have id "2", even though the HTML5
+    // spec implies it should be "1".  Minefield matches the HTML5 spec here.
+
+    ASSERT(element->isHTMLElement()); // otherwise localName() might be wrong.
+    AtomicHTMLToken fakeToken(HTMLToken::StartTag, element->localName(), cloneAttributes(element));
+    return createHTMLElement(fakeToken);
+}
+
 bool HTMLConstructionSite::indexOfFirstUnopenFormattingElement(unsigned& firstUnopenElementIndex) const
 {
     if (m_activeFormattingElements.isEmpty())
@@ -319,9 +360,8 @@ void HTMLConstructionSite::reconstructTheActiveFormattingElements()
     ASSERT(unopenEntryIndex < m_activeFormattingElements.size());
     for (; unopenEntryIndex < m_activeFormattingElements.size(); ++unopenEntryIndex) {
         HTMLFormattingElementList::Entry& unopenedEntry = m_activeFormattingElements.at(unopenEntryIndex);
-        // FIXME: We're supposed to save the original token in the entry.
-        AtomicHTMLToken fakeToken(HTMLToken::StartTag, unopenedEntry.element()->localName());
-        insertHTMLElement(fakeToken);
+        RefPtr<Element> reconstructed = createHTMLElementFromSavedElement(unopenedEntry.element());
+        m_openElements.push(attachToCurrent(reconstructed.release()));
         unopenedEntry.replaceElement(currentElement());
     }
 }
diff --git a/WebCore/html/HTMLConstructionSite.h b/WebCore/html/HTMLConstructionSite.h
index c0af9b3..d3a977a 100644
--- a/WebCore/html/HTMLConstructionSite.h
+++ b/WebCore/html/HTMLConstructionSite.h
@@ -64,6 +64,7 @@ public:
     void insertHTMLBodyStartTagInBody(AtomicHTMLToken&);
 
     PassRefPtr<Element> createHTMLElement(AtomicHTMLToken&);
+    PassRefPtr<Element> createHTMLElementFromElementRecord(HTMLElementStack::ElementRecord*);
 
     void fosterParent(Node*);
 
@@ -113,13 +114,14 @@ private:
 
     template<typename ChildType>
     PassRefPtr<ChildType> attach(Node* parent, PassRefPtr<ChildType> child);
+    PassRefPtr<Element> attachToCurrent(PassRefPtr<Element>);
 
     void attachAtSite(const AttachmentSite&, PassRefPtr<Node> child);
     void findFosterSite(AttachmentSite&);
 
+    PassRefPtr<Element> createHTMLElementFromSavedElement(Element*);
     PassRefPtr<Element> createElement(AtomicHTMLToken&, const AtomicString& namespaceURI);
 
-    PassRefPtr<Element> createHTMLElementAndAttachToCurrent(AtomicHTMLToken&);
     void mergeAttributesFromTokenIntoElement(AtomicHTMLToken&, Element*);
 
     Document* m_document;
diff --git a/WebCore/html/HTMLTreeBuilder.cpp b/WebCore/html/HTMLTreeBuilder.cpp
index fd61ed0..c524ae8 100644
--- a/WebCore/html/HTMLTreeBuilder.cpp
+++ b/WebCore/html/HTMLTreeBuilder.cpp
@@ -1675,11 +1675,7 @@ void HTMLTreeBuilder::callTheAdoptionAgency(AtomicHTMLToken& token)
             if (node == formattingElementRecord)
                 break;
             // 6.5
-            // FIXME: We're supposed to save the original token in the entry.
-            AtomicHTMLToken fakeToken(HTMLToken::StartTag, node->element()->localName());
-            // Is createHTMLElement correct? (instead of insertHTMLElement)
-            // Does this code ever leave newElement unattached?
-            RefPtr<Element> newElement = m_tree.createHTMLElement(fakeToken);
+            RefPtr<Element> newElement = m_tree.createHTMLElementFromElementRecord(node);
             HTMLFormattingElementList::Entry* nodeEntry = m_tree.activeFormattingElements()->find(node->element());
             nodeEntry->replaceElement(newElement.get());
             node->replaceElement(newElement.release());
@@ -1708,9 +1704,7 @@ void HTMLTreeBuilder::callTheAdoptionAgency(AtomicHTMLToken& token)
             ASSERT(!ec);
         }
         // 8
-        // FIXME: We're supposed to save the original token in the entry.
-        AtomicHTMLToken fakeToken(HTMLToken::StartTag, formattingElement->localName());
-        RefPtr<Element> newElement = m_tree.createHTMLElement(fakeToken);
+        RefPtr<Element> newElement = m_tree.createHTMLElementFromElementRecord(formattingElementRecord);
         // 9
         reparentChildren(furthestBlock->element(), newElement.get());
         // 10

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list