Document.createNodeIterator(null) / Document.createTreeWalker(null) should throw...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Sep 2015 00:30:03 +0000 (00:30 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Sep 2015 00:30:03 +0000 (00:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149126
<rdar://problem/22564891>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline W3C test now that a new check is passing.

* web-platform-tests/dom/traversal/TreeWalker-basic-expected.txt:

Source/WebCore:

Document.createNodeIterator(null) / Document.createTreeWalker(null)
should throw a TypeError:
https://dom.spec.whatwg.org/#interface-document

This is because the parameter is not nullable and Web IDL says we
should throw a TypeError in this case.

Firefox and Chrome throw an exception in this case. This patch
aligns our behavior with the specification and other major browsers.

No new tests, already covered by existing W3C test.

* dom/Document.cpp:
(WebCore::Document::createNodeIterator):
(WebCore::Document::createTreeWalker):
* dom/Document.h:
* dom/Document.idl:
* dom/NodeIterator.cpp:
(WebCore::NodeIterator::NodeIterator):
* dom/NodeIterator.h:
(WebCore::NodeIterator::create):
* dom/Traversal.cpp:
(WebCore::NodeIteratorBase::NodeIteratorBase):
* dom/Traversal.h:
* dom/TreeWalker.cpp:
(WebCore::TreeWalker::TreeWalker):
* dom/TreeWalker.h:
(WebCore::TreeWalker::create):

LayoutTests:

Update existing tests to add test coverage for this case.

* fast/dom/createNodeIterator-parameters-expected.txt:
* fast/dom/createNodeIterator-parameters.html:
* fast/dom/createTreeWalker-parameters-expected.txt:
* fast/dom/createTreeWalker-parameters.html:

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/createNodeIterator-parameters-expected.txt
LayoutTests/fast/dom/createNodeIterator-parameters.html
LayoutTests/fast/dom/createTreeWalker-parameters-expected.txt
LayoutTests/fast/dom/createTreeWalker-parameters.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-basic-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Document.idl
Source/WebCore/dom/NodeIterator.cpp
Source/WebCore/dom/NodeIterator.h
Source/WebCore/dom/Traversal.cpp
Source/WebCore/dom/Traversal.h
Source/WebCore/dom/TreeWalker.cpp
Source/WebCore/dom/TreeWalker.h

index 0f9ec5d..2e33ea6 100644 (file)
@@ -1,5 +1,20 @@
 2015-09-14  Chris Dumez  <cdumez@apple.com>
 
+        Document.createNodeIterator(null) / Document.createTreeWalker(null) should throw a TypeError
+        https://bugs.webkit.org/show_bug.cgi?id=149126
+        <rdar://problem/22564891>
+
+        Reviewed by Ryosuke Niwa.
+
+        Update existing tests to add test coverage for this case.
+
+        * fast/dom/createNodeIterator-parameters-expected.txt:
+        * fast/dom/createNodeIterator-parameters.html:
+        * fast/dom/createTreeWalker-parameters-expected.txt:
+        * fast/dom/createTreeWalker-parameters.html:
+
+2015-09-14  Chris Dumez  <cdumez@apple.com>
+
         window.HTMLDetailsElement should exist
         https://bugs.webkit.org/show_bug.cgi?id=149139
 
index f815520..a949343 100644 (file)
@@ -3,9 +3,12 @@ Tests parameters of document.createNodeIterator() API.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-No parameters
+No parameters
 PASS document.createNodeIterator() threw exception TypeError: Not enough arguments.
 
+Null root node
+PASS document.createNodeIterator(null) threw exception TypeError: Type error.
+
 Default parameters
 iterator = document.createNodeIterator(document)
 PASS iterator.root is document
index 412b38a..d68149f 100644 (file)
@@ -5,9 +5,13 @@
 <script>
 description("Tests parameters of document.createNodeIterator() API.");
 
-debug("No parameters");
+debug("No parameters");
 shouldThrow("document.createNodeIterator()", "'TypeError: Not enough arguments'");
 
+debug("")
+debug("Null root node");
+shouldThrow("document.createNodeIterator(null)", "'TypeError: Type error'");
+
 debug("");
 debug("Default parameters");
 evalAndLog("iterator = document.createNodeIterator(document)");
index 03cf6cc..af6448a 100644 (file)
@@ -3,9 +3,12 @@ Tests parameters of document.createTreeWalker() API.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-No parameters
+No parameters
 PASS document.createTreeWalker() threw exception TypeError: Not enough arguments.
 
+Null root node
+PASS document.createTreeWalker(null) threw exception TypeError: Type error.
+
 Default parameters
 walker = document.createTreeWalker(document)
 PASS walker.root is document
index 13d4a4e..669f9a9 100644 (file)
@@ -5,9 +5,13 @@
 <script>
 description("Tests parameters of document.createTreeWalker() API.");
 
-debug("No parameters");
+debug("No parameters");
 shouldThrow("document.createTreeWalker()", "'TypeError: Not enough arguments'");
 
+debug("")
+debug("Null root node");
+shouldThrow("document.createTreeWalker(null)", "'TypeError: Type error'");
+
 debug("");
 debug("Default parameters");
 evalAndLog("walker = document.createTreeWalker(document)");
index 7bff58d..9261597 100644 (file)
@@ -1,5 +1,17 @@
 2015-09-14  Chris Dumez  <cdumez@apple.com>
 
+        Document.createNodeIterator(null) / Document.createTreeWalker(null) should throw a TypeError
+        https://bugs.webkit.org/show_bug.cgi?id=149126
+        <rdar://problem/22564891>
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline W3C test now that a new check is passing.
+
+        * web-platform-tests/dom/traversal/TreeWalker-basic-expected.txt:
+
+2015-09-14  Chris Dumez  <cdumez@apple.com>
+
         window.HTMLDetailsElement should exist
         https://bugs.webkit.org/show_bug.cgi?id=149139
 
index 5b476cf..cb7a767 100644 (file)
@@ -4,7 +4,7 @@ This test checks the basic functionality of TreeWalker.
 PASS Construct a TreeWalker by document.createTreeWalker(root). 
 PASS Construct a TreeWalker by document.createTreeWalker(root, null, null). 
 FAIL Construct a TreeWalker by document.createTreeWalker(root, undefined, undefined). assert_equals: whatToShow expected 4294967295 but got 0
-FAIL Give an invalid root node to document.createTreeWalker(). assert_throws: function "function () { document.createTreeWalker(null); }" did not throw
+PASS Give an invalid root node to document.createTreeWalker(). 
 PASS Walk over nodes. 
 PASS Optional arguments to createTreeWalker should be optional (3 passed, null). 
 
index c548def..c2b7def 100644 (file)
@@ -1,3 +1,40 @@
+2015-09-14  Chris Dumez  <cdumez@apple.com>
+
+        Document.createNodeIterator(null) / Document.createTreeWalker(null) should throw a TypeError
+        https://bugs.webkit.org/show_bug.cgi?id=149126
+        <rdar://problem/22564891>
+
+        Reviewed by Ryosuke Niwa.
+
+        Document.createNodeIterator(null) / Document.createTreeWalker(null)
+        should throw a TypeError:
+        https://dom.spec.whatwg.org/#interface-document
+
+        This is because the parameter is not nullable and Web IDL says we
+        should throw a TypeError in this case.
+
+        Firefox and Chrome throw an exception in this case. This patch
+        aligns our behavior with the specification and other major browsers.
+
+        No new tests, already covered by existing W3C test.
+
+        * dom/Document.cpp:
+        (WebCore::Document::createNodeIterator):
+        (WebCore::Document::createTreeWalker):
+        * dom/Document.h:
+        * dom/Document.idl:
+        * dom/NodeIterator.cpp:
+        (WebCore::NodeIterator::NodeIterator):
+        * dom/NodeIterator.h:
+        (WebCore::NodeIterator::create):
+        * dom/Traversal.cpp:
+        (WebCore::NodeIteratorBase::NodeIteratorBase):
+        * dom/Traversal.h:
+        * dom/TreeWalker.cpp:
+        (WebCore::TreeWalker::TreeWalker):
+        * dom/TreeWalker.h:
+        (WebCore::TreeWalker::create):
+
 2015-09-14  Alex Christensen  <achristensen@webkit.org>
 
         Fix Windows clean build after r189746
index 7f96535..be326f0 100644 (file)
@@ -1720,14 +1720,52 @@ Ref<Range> Document::createRange()
     return Range::create(*this);
 }
 
-RefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned long whatToShow, PassRefPtr<NodeFilter> filter, bool expandEntityReferences)
+RefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, bool expandEntityReferences, ExceptionCode& ec)
 {
-    return NodeIterator::create(root, whatToShow, filter, expandEntityReferences);
+    if (!root) {
+        ec = TypeError;
+        return nullptr;
+    }
+    return NodeIterator::create(root, whatToShow, WTF::move(filter), expandEntityReferences);
+}
+
+RefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, ExceptionCode& ec)
+{
+    return createNodeIterator(root, whatToShow, WTF::move(filter), false, ec);
+}
+
+RefPtr<NodeIterator> Document::createNodeIterator(Node* root, unsigned long whatToShow, ExceptionCode& ec)
+{
+    return createNodeIterator(root, whatToShow, nullptr, false, ec);
+}
+
+RefPtr<NodeIterator> Document::createNodeIterator(Node* root, ExceptionCode& ec)
+{
+    return createNodeIterator(root, 0xFFFFFFFF, nullptr, false, ec);
+}
+
+RefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, bool expandEntityReferences, ExceptionCode& ec)
+{
+    if (!root) {
+        ec = TypeError;
+        return nullptr;
+    }
+    return TreeWalker::create(root, whatToShow, WTF::move(filter), expandEntityReferences);
+}
+
+RefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, ExceptionCode& ec)
+{
+    return createTreeWalker(root, whatToShow, WTF::move(filter), false, ec);
+}
+
+RefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned long whatToShow, ExceptionCode& ec)
+{
+    return createTreeWalker(root, whatToShow, nullptr, false, ec);
 }
 
-RefPtr<TreeWalker> Document::createTreeWalker(Node* root, unsigned long whatToShow, PassRefPtr<NodeFilter> filter, bool expandEntityReferences)
+RefPtr<TreeWalker> Document::createTreeWalker(Node* root, ExceptionCode& ec)
 {
-    return TreeWalker::create(root, whatToShow, filter, expandEntityReferences);
+    return createTreeWalker(root, 0xFFFFFFFF, nullptr, false, ec);
 }
 
 void Document::scheduleForcedStyleRecalc()
index a000b6c..88d3466 100644 (file)
@@ -533,9 +533,15 @@ public:
 
     WEBCORE_EXPORT Ref<Range> createRange();
 
-    RefPtr<NodeIterator> createNodeIterator(Node* root, unsigned long whatToShow = 0xFFFFFFFF, PassRefPtr<NodeFilter> = nullptr, bool expandEntityReferences = false);
-
-    RefPtr<TreeWalker> createTreeWalker(Node* root, unsigned long whatToShow = 0xFFFFFFFF, PassRefPtr<NodeFilter> = nullptr, bool expandEntityReferences = false);
+    RefPtr<NodeIterator> createNodeIterator(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&&, bool expandEntityReferences, ExceptionCode&);
+    RefPtr<NodeIterator> createNodeIterator(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&&, ExceptionCode&);
+    RefPtr<NodeIterator> createNodeIterator(Node* root, unsigned long whatToShow, ExceptionCode&);
+    RefPtr<NodeIterator> createNodeIterator(Node* root, ExceptionCode&);
+
+    RefPtr<TreeWalker> createTreeWalker(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&&, bool expandEntityReferences, ExceptionCode&);
+    RefPtr<TreeWalker> createTreeWalker(Node* root, unsigned long whatToShow, RefPtr<NodeFilter>&&, ExceptionCode&);
+    RefPtr<TreeWalker> createTreeWalker(Node* root, unsigned long whatToShow, ExceptionCode&);
+    RefPtr<TreeWalker> createTreeWalker(Node* root, ExceptionCode&);
 
     // Special support for editing
     Ref<CSSStyleDeclaration> createCSSStyleDeclaration();
index a9e18d3..391c983 100644 (file)
 
     // DOM Level 2 Tranversal and Range (DocumentTraversal interface)
 
-    [ObjCLegacyUnnamedParameters] NodeIterator createNodeIterator(Node root,
+    [ObjCLegacyUnnamedParameters, RaisesException] NodeIterator createNodeIterator(Node root,
         optional unsigned long whatToShow,
         optional NodeFilter? filter,
         optional boolean expandEntityReferences);
-    [ObjCLegacyUnnamedParameters] TreeWalker createTreeWalker(Node root,
+    [ObjCLegacyUnnamedParameters, RaisesException] TreeWalker createTreeWalker(Node root,
         optional unsigned long whatToShow,
         optional NodeFilter? filter,
         optional boolean expandEntityReferences);
index 8f29ab0..48a0a19 100644 (file)
@@ -76,8 +76,8 @@ bool NodeIterator::NodePointer::moveToPrevious(Node* root)
     return node;
 }
 
-NodeIterator::NodeIterator(PassRefPtr<Node> rootNode, unsigned long whatToShow, PassRefPtr<NodeFilter> filter, bool expandEntityReferences)
-    : NodeIteratorBase(rootNode, whatToShow, filter, expandEntityReferences)
+NodeIterator::NodeIterator(PassRefPtr<Node> rootNode, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, bool expandEntityReferences)
+    : NodeIteratorBase(rootNode, whatToShow, WTF::move(filter), expandEntityReferences)
     , m_referenceNode(root(), true)
 {
     root()->document().attachNodeIterator(this);
index 49766ad..9719274 100644 (file)
@@ -37,9 +37,9 @@ namespace WebCore {
 
     class NodeIterator : public ScriptWrappable, public RefCounted<NodeIterator>, public NodeIteratorBase {
     public:
-        static Ref<NodeIterator> create(PassRefPtr<Node> rootNode, unsigned long whatToShow, PassRefPtr<NodeFilter> filter, bool expandEntityReferences)
+        static Ref<NodeIterator> create(PassRefPtr<Node> rootNode, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, bool expandEntityReferences)
         {
-            return adoptRef(*new NodeIterator(rootNode, whatToShow, filter, expandEntityReferences));
+            return adoptRef(*new NodeIterator(rootNode, whatToShow, WTF::move(filter), expandEntityReferences));
         }
         ~NodeIterator();
 
@@ -54,7 +54,7 @@ namespace WebCore {
         void nodeWillBeRemoved(Node&);
 
     private:
-        NodeIterator(PassRefPtr<Node>, unsigned long whatToShow, PassRefPtr<NodeFilter>, bool expandEntityReferences);
+        NodeIterator(PassRefPtr<Node>, unsigned long whatToShow, RefPtr<NodeFilter>&&, bool expandEntityReferences);
 
         struct NodePointer {
             RefPtr<Node> node;
index 763f141..74a928f 100644 (file)
 
 namespace WebCore {
 
-NodeIteratorBase::NodeIteratorBase(PassRefPtr<Node> rootNode, unsigned long whatToShow, PassRefPtr<NodeFilter> nodeFilter, bool expandEntityReferences)
+NodeIteratorBase::NodeIteratorBase(PassRefPtr<Node> rootNode, unsigned long whatToShow, RefPtr<NodeFilter>&& nodeFilter, bool expandEntityReferences)
     : m_root(rootNode)
     , m_whatToShow(whatToShow)
-    , m_filter(nodeFilter)
+    , m_filter(WTF::move(nodeFilter))
     , m_expandEntityReferences(expandEntityReferences)
 {
 }
index 6acfbdb..ed6e8dd 100644 (file)
@@ -41,7 +41,7 @@ namespace WebCore {
         bool expandEntityReferences() const { return m_expandEntityReferences; }
 
     protected:
-        NodeIteratorBase(PassRefPtr<Node>, unsigned long whatToShow, PassRefPtr<NodeFilter>, bool expandEntityReferences);
+        NodeIteratorBase(PassRefPtr<Node>, unsigned long whatToShow, RefPtr<NodeFilter>&&, bool expandEntityReferences);
         short acceptNode(Node*) const;
 
     private:
index 0583620..ad0ace1 100644 (file)
@@ -33,8 +33,8 @@
 
 namespace WebCore {
 
-TreeWalker::TreeWalker(PassRefPtr<Node> rootNode, unsigned long whatToShow, PassRefPtr<NodeFilter> filter, bool expandEntityReferences)
-    : NodeIteratorBase(rootNode, whatToShow, filter, expandEntityReferences)
+TreeWalker::TreeWalker(PassRefPtr<Node> rootNode, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, bool expandEntityReferences)
+    : NodeIteratorBase(rootNode, whatToShow, WTF::move(filter), expandEntityReferences)
     , m_current(root())
 {
 }
index 6a3fe71..bf69bde 100644 (file)
@@ -37,9 +37,9 @@ namespace WebCore {
 
     class TreeWalker : public ScriptWrappable, public RefCounted<TreeWalker>, public NodeIteratorBase {
     public:
-        static Ref<TreeWalker> create(PassRefPtr<Node> rootNode, unsigned long whatToShow, PassRefPtr<NodeFilter> filter, bool expandEntityReferences)
+        static Ref<TreeWalker> create(PassRefPtr<Node> rootNode, unsigned long whatToShow, RefPtr<NodeFilter>&& filter, bool expandEntityReferences)
         {
-            return adoptRef(*new TreeWalker(rootNode, whatToShow, filter, expandEntityReferences));
+            return adoptRef(*new TreeWalker(rootNode, whatToShow, WTF::move(filter), expandEntityReferences));
         }                            
 
         Node* currentNode() const { return m_current.get(); }
@@ -54,7 +54,7 @@ namespace WebCore {
         Node* nextNode();
 
     private:
-        TreeWalker(PassRefPtr<Node>, unsigned long whatToShow, PassRefPtr<NodeFilter>, bool expandEntityReferences);
+        TreeWalker(PassRefPtr<Node>, unsigned long whatToShow, RefPtr<NodeFilter>&&, bool expandEntityReferences);
         
         Node* setCurrent(PassRefPtr<Node>);