Summary: | Document.createNodeIterator(null) / Document.createTreeWalker(null) should throw a TypeError | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, commit-queue, rniwa, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar, WebExposed | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | https://dom.spec.whatwg.org/#interface-document | ||||||||
Attachments: |
|
Description
Chris Dumez
2015-09-14 13:59:47 PDT
Created attachment 261134 [details]
Patch
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 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. Created attachment 261152 [details]
Patch
Comment on attachment 261152 [details] Patch Clearing flags on attachment: 261152 Committed r189765: <http://trac.webkit.org/changeset/189765> All reviewed patches have been landed. Closing bug. 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. |