Title: [154441] trunk
Revision
154441
Author
rwlb...@webkit.org
Date
2013-08-22 04:50:44 -0700 (Thu, 22 Aug 2013)

Log Message

REGRESSION: Assertion failure !collection->hasExactlyOneItem() in WebCore::namedItemGetter
https://bugs.webkit.org/show_bug.cgi?id=118056

Reviewed by Ryosuke Niwa.

Source/WebCore:

The assert is hit in Debug mode because the DocumentOrderedMap in HTMLDocument::m_windowNamedItem
includes matched SVG elements, but the WindowNameCollection used to collect the elements does not.
This means the HTMLCollection stripped of SVG elements could end up hitting the empty or single item
assertion, which the testcase verifies.
To fix this change WindowNameCollection to include both SVG and HTML elements so it matches DocumentOrderedMap.

Tests: svg/custom/document-all-includes-svg.html
       svg/custom/window-named-item-lookup.html

* html/HTMLCollection.cpp:
(WebCore::isMatchingElement):
(WebCore::HTMLCollection::updateNameCache):

LayoutTests:

Add test to verify that SVG and HTML elements with id's are both found when using document.all or named properties.

* svg/custom/document-all-includes-svg-expected.txt: Added.
* svg/custom/document-all-includes-svg.html: Added.
* svg/custom/window-named-item-lookup-expected.txt: Added.
* svg/custom/window-named-item-lookup.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (154440 => 154441)


--- trunk/LayoutTests/ChangeLog	2013-08-22 11:43:25 UTC (rev 154440)
+++ trunk/LayoutTests/ChangeLog	2013-08-22 11:50:44 UTC (rev 154441)
@@ -1,3 +1,17 @@
+2013-08-22  Rob Buis  <rwlb...@webkit.org>
+
+        REGRESSION: Assertion failure !collection->hasExactlyOneItem() in WebCore::namedItemGetter
+        https://bugs.webkit.org/show_bug.cgi?id=118056
+
+        Reviewed by Ryosuke Niwa.
+
+        Add test to verify that SVG and HTML elements with id's are both found when using document.all or named properties.
+
+        * svg/custom/document-all-includes-svg-expected.txt: Added.
+        * svg/custom/document-all-includes-svg.html: Added.
+        * svg/custom/window-named-item-lookup-expected.txt: Added.
+        * svg/custom/window-named-item-lookup.html: Added.
+
 2013-08-22  Renata Hodovan  <r...@webkit.org>
 
         ASSERTION FAILED: extractedStyle in WebCore::ApplyStyleCommand::removeInlineStyleFromElement

Added: trunk/LayoutTests/svg/custom/document-all-includes-svg-expected.txt (0 => 154441)


--- trunk/LayoutTests/svg/custom/document-all-includes-svg-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/document-all-includes-svg-expected.txt	2013-08-22 11:50:44 UTC (rev 154441)
@@ -0,0 +1,9 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS document.all['test1'].length is 2
+PASS document.all['test2'].length is 2
+PASS document.all['test3'].nodeName is "A"
+PASS document.all['test4'].length is 2
+PASS document.all['test5'].length is 2
+

Added: trunk/LayoutTests/svg/custom/document-all-includes-svg.html (0 => 154441)


--- trunk/LayoutTests/svg/custom/document-all-includes-svg.html	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/document-all-includes-svg.html	2013-08-22 11:50:44 UTC (rev 154441)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script src=""
+    <script>
+      function runTest()
+      {
+        shouldBe("document.all['test1'].length", "2");
+        shouldBe("document.all['test2'].length", "2");
+        shouldBeEqualToString("document.all['test3'].nodeName", 'A');
+        shouldBe("document.all['test4'].length", "2");
+        shouldBe("document.all['test5'].length", "2");
+      }
+     </script>
+     <script src=""
+  </head>
+  <body _onload_="runTest()">
+    <a id="test1"></a>
+    <svg id="test1"/>
+
+    <svg id="test2"/>
+    <a id="test2"></a>
+
+    <a id="test3"></a>
+
+    <a id="test4"></a>
+    <div id="test4"></div>
+
+    <svg id="test5"></svg>
+    <svg id="test5"></svg>
+
+  </body>
+</html>

Added: trunk/LayoutTests/svg/custom/window-named-item-lookup-expected.txt (0 => 154441)


--- trunk/LayoutTests/svg/custom/window-named-item-lookup-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/window-named-item-lookup-expected.txt	2013-08-22 11:50:44 UTC (rev 154441)
@@ -0,0 +1,9 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS window.test1.length is 2
+PASS window.test2.length is 2
+PASS window.test3.nodeName is "A"
+PASS window.test4.length is 2
+PASS window.test5.length is 2
+

Added: trunk/LayoutTests/svg/custom/window-named-item-lookup.html (0 => 154441)


--- trunk/LayoutTests/svg/custom/window-named-item-lookup.html	                        (rev 0)
+++ trunk/LayoutTests/svg/custom/window-named-item-lookup.html	2013-08-22 11:50:44 UTC (rev 154441)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script src=""
+    <script>
+      function runTest()
+      {
+        shouldBe("window.test1.length", "2");
+        shouldBe("window.test2.length", "2");
+        shouldBeEqualToString("window.test3.nodeName", 'A');
+        shouldBe("window.test4.length", "2");
+        shouldBe("window.test5.length", "2");
+      }
+     </script>
+     <script src=""
+  </head>
+  <body _onload_="runTest()">
+    <a id="test1"></a>
+    <svg id="test1"/>
+
+    <svg id="test2"/>
+    <a id="test2"></a>
+
+    <a id="test3"></a>
+
+    <a id="test4"></a>
+    <div id="test4"></div>
+
+    <svg id="test5"></svg>
+    <svg id="test5"></svg>
+
+  </body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (154440 => 154441)


--- trunk/Source/WebCore/ChangeLog	2013-08-22 11:43:25 UTC (rev 154440)
+++ trunk/Source/WebCore/ChangeLog	2013-08-22 11:50:44 UTC (rev 154441)
@@ -1,3 +1,23 @@
+2013-08-22  Rob Buis  <rwlb...@webkit.org>
+
+        REGRESSION: Assertion failure !collection->hasExactlyOneItem() in WebCore::namedItemGetter
+        https://bugs.webkit.org/show_bug.cgi?id=118056
+
+        Reviewed by Ryosuke Niwa.
+
+        The assert is hit in Debug mode because the DocumentOrderedMap in HTMLDocument::m_windowNamedItem
+        includes matched SVG elements, but the WindowNameCollection used to collect the elements does not.
+        This means the HTMLCollection stripped of SVG elements could end up hitting the empty or single item
+        assertion, which the testcase verifies.
+        To fix this change WindowNameCollection to include both SVG and HTML elements so it matches DocumentOrderedMap.
+
+        Tests: svg/custom/document-all-includes-svg.html
+               svg/custom/window-named-item-lookup.html
+
+        * html/HTMLCollection.cpp:
+        (WebCore::isMatchingElement):
+        (WebCore::HTMLCollection::updateNameCache):
+
 2013-08-22  Gyuyoung Kim  <gyuyoung....@samsung.com>
 
         Introduce toSVGRadialGradientElement(), and use it

Modified: trunk/Source/WebCore/html/HTMLCollection.cpp (154440 => 154441)


--- trunk/Source/WebCore/html/HTMLCollection.cpp	2013-08-22 11:43:25 UTC (rev 154440)
+++ trunk/Source/WebCore/html/HTMLCollection.cpp	2013-08-22 11:50:44 UTC (rev 154441)
@@ -182,7 +182,7 @@
 template <> inline bool isMatchingElement(const HTMLCollection* htmlCollection, Element* element)
 {
     CollectionType type = htmlCollection->type();
-    if (!element->isHTMLElement() && !(type == DocAll || type == NodeChildren))
+    if (!element->isHTMLElement() && !(type == DocAll || type == NodeChildren || type == WindowNamedItems))
         return false;
 
     switch (type) {
@@ -627,15 +627,14 @@
 
     unsigned arrayOffset = 0;
     for (Element* element = traverseFirstElement(arrayOffset, root); element; element = traverseNextElement(arrayOffset, element, root)) {
+        const AtomicString& idAttrVal = element->getIdAttribute();
+        if (!idAttrVal.isEmpty())
+            appendIdCache(idAttrVal, element);
         if (!element->isHTMLElement())
             continue;
-        HTMLElement* htmlElement = toHTMLElement(element);
-        const AtomicString& idAttrVal = htmlElement->getIdAttribute();
-        const AtomicString& nameAttrVal = htmlElement->getNameAttribute();
-        if (!idAttrVal.isEmpty())
-            appendIdCache(idAttrVal, htmlElement);
-        if (!nameAttrVal.isEmpty() && idAttrVal != nameAttrVal && (type() != DocAll || nameShouldBeVisibleInDocumentAll(htmlElement)))
-            appendNameCache(nameAttrVal, htmlElement);
+        const AtomicString& nameAttrVal = element->getNameAttribute();
+        if (!nameAttrVal.isEmpty() && idAttrVal != nameAttrVal && (type() != DocAll || nameShouldBeVisibleInDocumentAll(toHTMLElement(element))))
+            appendNameCache(nameAttrVal, element);
     }
 
     setHasNameCache();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to