Bug 149126 - Document.createNodeIterator(null) / Document.createTreeWalker(null) should throw a TypeError
Summary: Document.createNodeIterator(null) / Document.createTreeWalker(null) should th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://dom.spec.whatwg.org/#interfac...
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2015-09-14 13:59 PDT by Chris Dumez
Modified: 2015-09-14 17:33 PDT (History)
4 users (show)

See Also:


Attachments
Patch (14.63 KB, patch)
2015-09-14 15:07 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (18.75 KB, patch)
2015-09-14 16:40 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-09-14 13:59:47 PDT
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.
Comment 1 Chris Dumez 2015-09-14 14:00:15 PDT
rdar://problem/22564891
Comment 2 Chris Dumez 2015-09-14 15:07:21 PDT
Created attachment 261134 [details]
Patch
Comment 3 Ryosuke Niwa 2015-09-14 15:57:10 PDT
Comment on attachment 261134 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261134&action=review

> Source/WebCore/dom/Document.h:540
> +    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&);
> +

We can't rely on default arguments!?
Can we somehow fix the bidding code generator to take care of this instead?

> LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-basic-expected.txt:7
> -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(). 

Does this test all the combinations?
If not, it might be worth adding a new test case.
Comment 4 Chris Dumez 2015-09-14 16:05:44 PDT
Comment on attachment 261134 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261134&action=review

>> Source/WebCore/dom/Document.h:540
>> +
> 
> We can't rely on default arguments!?
> Can we somehow fix the bidding code generator to take care of this instead?

Well, the issue is that the function now takes an ExceptionCode& so we cannot use default arguments. Longer term, my plan is to use WTF::Optional for optional parameters. This will probably be needed when we supposed passing undefined for optional parameters, e.g.
document.createNodeIterator(node, undefined, filter, false);

In this case, we should use the default value for whatToShow parameter.

>> LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-basic-expected.txt:7
>> +PASS Give an invalid root node to document.createTreeWalker(). 
> 
> Does this test all the combinations?
> If not, it might be worth adding a new test case.

All combinations? I only changed the behavior of the first argument of createNodeIterator() / createTreeWalker().
Now that I look at it though, there is no coverage for createNodeIterator(null) but I'll add a test before landing.
Comment 5 Chris Dumez 2015-09-14 16:40:24 PDT
Created attachment 261152 [details]
Patch
Comment 6 WebKit Commit Bot 2015-09-14 17:30:17 PDT
Comment on attachment 261152 [details]
Patch

Clearing flags on attachment: 261152

Committed r189765: <http://trac.webkit.org/changeset/189765>
Comment 7 WebKit Commit Bot 2015-09-14 17:30:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Joseph Pecoraro 2015-09-14 17:33:14 PDT
Comment on attachment 261152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261152&action=review

> Source/WebCore/dom/Document.cpp:1744
> +    return createNodeIterator(root, 0xFFFFFFFF, nullptr, false, ec);

Would have been nice if this 0xFFFFFFFF was a constant somewhere. It is used in a couple places.