Attachment 261833 Details for Bug 149493 – Patch

[patch] Patch

bug-149493-20150923113835.patch (text/plain), 11.37 KB, created by Chris Dumez on 2015-09-23 11:38:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2015-09-23 11:38:55 PDT

Size: 11.37 KB

patch

obsolete

>Subversion Revision: 190177
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b2f5230ee6e817b4d2d378966f5d7c592b3b4e69..c9c441a5126aa285f3b35e519750895272e0031e 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2015-09-23  Chris Dumez  <cdumez@apple.com>
>+
>+        TreeWalker.previousSibling()  / nextSibling() does not behave according to the specification
>+        https://bugs.webkit.org/show_bug.cgi?id=149493
>+
>+        Reviewed by Darin Adler.
>+
>+        TreeWalker.previousSibling()  / nextSibling() does not behave according
>+        to the specification:
>+        - https://dom.spec.whatwg.org/#dom-treewalker-nextsibling
>+        - https://dom.spec.whatwg.org/#dom-treewalker-previoussibling
>+        - https://dom.spec.whatwg.org/#concept-traverse-siblings
>+
>+        In particular, the previous code would fail to update 'node' variable
>+        in the case acceptNode() returned FILTER_REJECT. This patch fixes this
>+        and refactors the function to match more closely the text of the spec
>+        and avoid code duplication.
>+
>+        No new tests, already covered by existing test.
>+
>+        * dom/TreeWalker.cpp:
>+        (WebCore::TreeWalker::traverseSiblings):
>+        (WebCore::TreeWalker::previousSibling):
>+        (WebCore::TreeWalker::nextSibling):
>+        (WebCore::TreeWalker::previousNode): Deleted.
>+        * dom/TreeWalker.h:
>+
> 2015-09-23  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         Mac production build fix attempt after r190155.
>diff --git a/Source/WebCore/dom/TreeWalker.cpp b/Source/WebCore/dom/TreeWalker.cpp
>index 877482aff47a3c5434af7532e6dc053d1513cd1d..ce7f1c189944bae6a982f7b4d2dc4613b6a1ae22 100644
>--- a/Source/WebCore/dom/TreeWalker.cpp
>+++ b/Source/WebCore/dom/TreeWalker.cpp
>@@ -130,29 +130,24 @@ Node* TreeWalker::lastChild()
>     return nullptr;
> }
> 
>-Node* TreeWalker::previousSibling()
>+template<TreeWalker::SiblingTraversalType type> Node* TreeWalker::traverseSiblings()
> {
>     RefPtr<Node> node = m_current;
>     if (node == root())
>         return nullptr;
>-    while (1) {
>-        for (RefPtr<Node> sibling = node->previousSibling(); sibling; ) {
>+
>+    auto isNext = type == SiblingTraversalType::Next;
>+    while (true) {
>+        for (RefPtr<Node> sibling = isNext ? node->nextSibling() : node->previousSibling(); sibling; ) {
>             short acceptNodeResult = acceptNode(sibling.get());
>-            switch (acceptNodeResult) {
>-                case NodeFilter::FILTER_ACCEPT:
>-                    m_current = sibling.release();
>-                    return m_current.get();
>-                case NodeFilter::FILTER_SKIP:
>-                    if (sibling->lastChild()) {
>-                        sibling = sibling->lastChild();
>-                        node = sibling;
>-                        continue;
>-                    }
>-                    break;
>-                case NodeFilter::FILTER_REJECT:
>-                    break;
>+            if (acceptNodeResult == NodeFilter::FILTER_ACCEPT) {
>+                m_current = WTF::move(sibling);
>+                return m_current.get();
>             }
>-            sibling = sibling->previousSibling();
>+            node = sibling;
>+            sibling = isNext ? sibling->firstChild() : sibling->lastChild();
>+            if (acceptNodeResult == NodeFilter::FILTER_REJECT || !sibling)
>+                sibling = isNext ? node->nextSibling() : node->previousSibling();
>         }
>         node = node->parentNode();
>         if (!node || node == root())
>@@ -163,37 +158,14 @@ Node* TreeWalker::previousSibling()
>     }
> }
> 
>+Node* TreeWalker::previousSibling()
>+{
>+    return traverseSiblings<SiblingTraversalType::Previous>();
>+}
>+
> Node* TreeWalker::nextSibling()
> {
>-    RefPtr<Node> node = m_current;
>-    if (node == root())
>-        return nullptr;
>-    while (1) {
>-        for (RefPtr<Node> sibling = node->nextSibling(); sibling; ) {
>-            short acceptNodeResult = acceptNode(sibling.get());
>-            switch (acceptNodeResult) {
>-                case NodeFilter::FILTER_ACCEPT:
>-                    m_current = sibling.release();
>-                    return m_current.get();
>-                case NodeFilter::FILTER_SKIP:
>-                    if (sibling->firstChild()) {
>-                        sibling = sibling->firstChild();
>-                        node = sibling;
>-                        continue;
>-                    }
>-                    break;
>-                case NodeFilter::FILTER_REJECT:
>-                    break;
>-            }
>-            sibling = sibling->nextSibling();
>-        }
>-        node = node->parentNode();
>-        if (!node || node == root())
>-            return nullptr;
>-        short acceptNodeResult = acceptNode(node.get());
>-        if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
>-            return nullptr;
>-    }
>+    return traverseSiblings<SiblingTraversalType::Next>();
> }
> 
> Node* TreeWalker::previousNode()
>diff --git a/Source/WebCore/dom/TreeWalker.h b/Source/WebCore/dom/TreeWalker.h
>index 228f23d4746a2159bb50bf6aa62788b73364fb9f..795eed133ac4202beda396f81d46c3cd99577c2b 100644
>--- a/Source/WebCore/dom/TreeWalker.h
>+++ b/Source/WebCore/dom/TreeWalker.h
>@@ -55,6 +55,8 @@ namespace WebCore {
> 
>     private:
>         TreeWalker(PassRefPtr<Node>, unsigned long whatToShow, RefPtr<NodeFilter>&&);
>+        enum class SiblingTraversalType { Previous, Next };
>+        template<SiblingTraversalType> Node* traverseSiblings();
>         
>         Node* setCurrent(PassRefPtr<Node>);
> 
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index b336896514d01f5cc9fd703258eac8400bf83bf9..59413b2136da5285326e88c793cd0607e25d86b9 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,5 +1,16 @@
> 2015-09-23  Chris Dumez  <cdumez@apple.com>
> 
>+        TreeWalker.previousSibling()  / nextSibling() does not behave according to the specification
>+        https://bugs.webkit.org/show_bug.cgi?id=149493
>+
>+        Reviewed by Darin Adler.
>+
>+        Rebaseline existing W3C DOM test now that more checks are passing.
>+
>+        * web-platform-tests/dom/traversal/TreeWalker-expected.txt:
>+
>+2015-09-23  Chris Dumez  <cdumez@apple.com>
>+
>         Range's setStartBefore() / setStartAfter() / setEndBefore() / setEndAfter() do not match the specification
>         https://bugs.webkit.org/show_bug.cgi?id=149490
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt
>index 655784db5bb28b8110723f9c543da6f9beec9078..f26aae84e314add726f2d46fcc401ec241dd609c 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt
>@@ -146,7 +146,7 @@ PASS document.createTreeWalker(document, 0, (function(node) { return node.nodeNa
> PASS document.createTreeWalker(document, 0xFFFFFFFF, null) 
> PASS document.createTreeWalker(document, 0xFFFFFFFF, (function(node) { return true })) 
> PASS document.createTreeWalker(document, 0xFFFFFFFF, (function(node) { return false })) 
>-FAIL document.createTreeWalker(document, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) assert_equals: .nextSibling() expected Text node "TreeWalker tests" but got null
>+PASS document.createTreeWalker(document, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) 
> PASS document.createTreeWalker(document, NodeFilter.SHOW_ELEMENT, null) 
> PASS document.createTreeWalker(document, NodeFilter.SHOW_ELEMENT, (function(node) { return true })) 
> PASS document.createTreeWalker(document, NodeFilter.SHOW_ELEMENT, (function(node) { return false })) 
>@@ -166,7 +166,7 @@ PASS document.createTreeWalker(detachedDiv, 0, (function(node) { return node.nod
> PASS document.createTreeWalker(detachedDiv, 0xFFFFFFFF, null) 
> PASS document.createTreeWalker(detachedDiv, 0xFFFFFFFF, (function(node) { return true })) 
> PASS document.createTreeWalker(detachedDiv, 0xFFFFFFFF, (function(node) { return false })) 
>-FAIL document.createTreeWalker(detachedDiv, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) assert_equals: .nextSibling() expected Text node "Wxyzabcd" but got null
>+PASS document.createTreeWalker(detachedDiv, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) 
> PASS document.createTreeWalker(detachedDiv, NodeFilter.SHOW_ELEMENT, null) 
> PASS document.createTreeWalker(detachedDiv, NodeFilter.SHOW_ELEMENT, (function(node) { return true })) 
> PASS document.createTreeWalker(detachedDiv, NodeFilter.SHOW_ELEMENT, (function(node) { return false })) 
>@@ -186,7 +186,7 @@ PASS document.createTreeWalker(foreignDoc, 0, (function(node) { return node.node
> PASS document.createTreeWalker(foreignDoc, 0xFFFFFFFF, null) 
> PASS document.createTreeWalker(foreignDoc, 0xFFFFFFFF, (function(node) { return true })) 
> PASS document.createTreeWalker(foreignDoc, 0xFFFFFFFF, (function(node) { return false })) 
>-FAIL document.createTreeWalker(foreignDoc, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) assert_equals: .nextSibling() expected Text node "Efghijkl" but got Comment node 
>+PASS document.createTreeWalker(foreignDoc, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) 
> PASS document.createTreeWalker(foreignDoc, NodeFilter.SHOW_ELEMENT, null) 
> PASS document.createTreeWalker(foreignDoc, NodeFilter.SHOW_ELEMENT, (function(node) { return true })) 
> PASS document.createTreeWalker(foreignDoc, NodeFilter.SHOW_ELEMENT, (function(node) { return false })) 
>@@ -226,7 +226,7 @@ PASS document.createTreeWalker(xmlDoc, 0, (function(node) { return node.nodeName
> PASS document.createTreeWalker(xmlDoc, 0xFFFFFFFF, null) 
> PASS document.createTreeWalker(xmlDoc, 0xFFFFFFFF, (function(node) { return true })) 
> PASS document.createTreeWalker(xmlDoc, 0xFFFFFFFF, (function(node) { return false })) 
>-FAIL document.createTreeWalker(xmlDoc, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) assert_equals: .nextSibling() expected Text node "do re mi fa so la ti" but got Comment node 
>+PASS document.createTreeWalker(xmlDoc, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) 
> PASS document.createTreeWalker(xmlDoc, NodeFilter.SHOW_ELEMENT, null) 
> PASS document.createTreeWalker(xmlDoc, NodeFilter.SHOW_ELEMENT, (function(node) { return true })) 
> PASS document.createTreeWalker(xmlDoc, NodeFilter.SHOW_ELEMENT, (function(node) { return false })) 
>@@ -506,8 +506,7 @@ PASS document.createTreeWalker(testDiv, 0, (function(node) { return node.nodeNam
> PASS document.createTreeWalker(testDiv, 0xFFFFFFFF, null) 
> PASS document.createTreeWalker(testDiv, 0xFFFFFFFF, (function(node) { return true })) 
> PASS document.createTreeWalker(testDiv, 0xFFFFFFFF, (function(node) { return false })) 
>-FAIL document.createTreeWalker(testDiv, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) assert_equals: .nextSibling() expected Text node "Ijklmnop
>-" but got Comment node 
>+PASS document.createTreeWalker(testDiv, 0xFFFFFFFF, (function(node) { return node.nodeName[0] == '#' })) 
> PASS document.createTreeWalker(testDiv, NodeFilter.SHOW_ELEMENT, null) 
> PASS document.createTreeWalker(testDiv, NodeFilter.SHOW_ELEMENT, (function(node) { return true })) 
> PASS document.createTreeWalker(testDiv, NodeFilter.SHOW_ELEMENT, (function(node) { return false }))

Actions: View | Formatted Diff | Diff

Attachments on bug 149493: 261826 | 261833