git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 7dc4e10)
REGRESSION(r150187): Safari fails to render allrecipe.com comment popups
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Aug 2013 02:16:56 +0000 (02:16 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Aug 2013 02:16:56 +0000 (02:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=119780

Reviewed by Benjamin Poulain.

Source/WebCore:

The bug was caused by SelectorDataList::executeFastPathForIdSelector not verifying that
elements found by getAllElementsById are descendents of rootNode when there are multiple
elements of the same id. This resulted in querySelector and querySelectorAll of an element
returning nodes outside of the element.

Fixed the bug by checking this condition when we have multiple elements of the same id.

Test: fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html

* dom/SelectorQuery.cpp:
(WebCore::SelectorDataList::executeFastPathForIdSelector):

LayoutTests:

* fast/selectors/querySelector-id-with-multiple-elements-with-same-id-expected.txt: Added.
* fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/dom/SelectorQuery.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 5b71b3532509378fc8a8c85f4409d0b511045b31..cdd63de3ede6050cbd344ed7cbb89658a145803a 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2013-08-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r150187): Safari fails to render allrecipe.com comment popups
+        https://bugs.webkit.org/show_bug.cgi?id=119780
+
+        Reviewed by Benjamin Poulain.
+
+        * fast/selectors/querySelector-id-with-multiple-elements-with-same-id-expected.txt: Added.
+        * fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html: Added.
+
 2013-08-13  Sam Weinig  <sam@webkit.org>
 
         [Re-land] Cleanup MediaQueryListListener
diff --git a/LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id-expected.txt b/LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id-expected.txt
new file mode 100644 (file)
index 0000000..48271b6
--- /dev/null
+++ b/LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id-expected.txt
@@ -0,0 +1,11 @@
+This test makes sure WebKit element.querySelector("#foo") returns a descendent node of the element even when there are multiple elements with the id foo.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS container.querySelectorAll("#foo").length is 1
+PASS container.querySelectorAll("#foo")[0] is container.firstChild
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html b/LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html
new file mode 100644 (file)
index 0000000..cd557df
--- /dev/null
+++ b/LayoutTests/fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<body>
+<span id="foo"></span>
+<div id="container"><span id="foo"></span></div>
+</body>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+description('This test makes sure WebKit element.querySelector("#foo") returns a descendent node of the element even when there are multiple elements with the id foo.');
+
+var container = document.getElementById('container');
+shouldBe('container.querySelectorAll("#foo").length', '1');
+shouldBe('container.querySelectorAll("#foo")[0]', 'container.firstChild');
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 493d80fdf8fe62578f367483ecd5e3fe3244736c..90aec8600881cadffd411b3086b934a1eabb20c0 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2013-08-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r150187): Safari fails to render allrecipe.com comment popups
+        https://bugs.webkit.org/show_bug.cgi?id=119780
+
+        Reviewed by Benjamin Poulain.
+
+        The bug was caused by SelectorDataList::executeFastPathForIdSelector not verifying that
+        elements found by getAllElementsById are descendents of rootNode when there are multiple
+        elements of the same id. This resulted in querySelector and querySelectorAll of an element
+        returning nodes outside of the element.
+
+        Fixed the bug by checking this condition when we have multiple elements of the same id.
+
+        Test: fast/selectors/querySelector-id-with-multiple-elements-with-same-id.html
+
+        * dom/SelectorQuery.cpp:
+        (WebCore::SelectorDataList::executeFastPathForIdSelector):
+
 2013-08-12  Ryosuke Niwa  <rniwa@webkit.org>
 
         Fix orphan needsLayout state in RenderTextControlSingleLine
diff --git a/Source/WebCore/dom/SelectorQuery.cpp b/Source/WebCore/dom/SelectorQuery.cpp
index 76cf7db8d53d65313c34afcabd742dac9db083ec..941b411b5d4049b74ae07e8cb4534522a6001e35 100644 (file)
--- a/Source/WebCore/dom/SelectorQuery.cpp
+++ b/Source/WebCore/dom/SelectorQuery.cpp
@@ -132,9 +132,10 @@ ALWAYS_INLINE void SelectorDataList::executeFastPathForIdSelector(const Node* ro
         const Vector<Element*>* elements = rootNode->treeScope()->getAllElementsById(idToMatch);
         ASSERT(elements);
         size_t count = elements->size();
+        bool rootNodeIsTreeScopeRoot = isTreeScopeRoot(rootNode);
         for (size_t i = 0; i < count; ++i) {
             Element* element = elements->at(i);
-            if (selectorMatches(selectorData, element, rootNode)) {
+            if ((rootNodeIsTreeScopeRoot || element->isDescendantOf(rootNode)) && selectorMatches(selectorData, element, rootNode)) {
                 matchedElements.append(element);
                 if (firstMatchOnly)
                     return;
Mirror of the WebKit open source project from svn.webkit.org.