RESOLVED FIXED 157457
Change HTMLSlotElement::assignedNodes to take an IDL dictionary instead of a WebCore::Dictionary
https://bugs.webkit.org/show_bug.cgi?id=157457
Summary Change HTMLSlotElement::assignedNodes to take an IDL dictionary instead of a ...
Darin Adler
Reported 2016-05-07 22:58:44 PDT
Change HTMLSlotElement::assignedNodes to take a IDL dictionary instead of a WebCore::Dictionary
Attachments
Patch (13.12 KB, patch)
2016-05-07 23:03 PDT, Darin Adler
no flags
Patch (7.12 KB, patch)
2016-05-08 10:50 PDT, Darin Adler
no flags
Patch (7.09 KB, patch)
2016-05-08 10:56 PDT, Darin Adler
cdumez: review+
Darin Adler
Comment 1 2016-05-07 23:03:16 PDT
Darin Adler
Comment 2 2016-05-08 08:28:13 PDT
Comment on attachment 278353 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278353&action=review > Source/WebCore/bindings/js/JSDOMConvert.h:54 > +template<> inline bool convert<bool>(JSC::ExecState& state, JSC::JSValue value) > +{ > + return value.toBoolean(&state); > +} I will rebase since I recently landed a patch that adds this so I don’t need to add it again. > Source/WebCore/html/HTMLSlotElement.cpp:113 > +Vector<Node*> HTMLSlotElement::assignedNodes(const Optional<AssignedNodesOptions>& options) const I’ll post a new patch that no longer uses Optional once I land the patch that fixes the use of Optional for dictionaries. > LayoutTests/fast/shadow-dom/HTMLSlotElement-interface.html:52 > -testSlotOutsideShadowTree(null); > +testSlotOutsideShadowTree({}); I won’t need any of these LayoutTests changes after fixing the handling of null in dictionaries, which is the patch I am working on right now.
Darin Adler
Comment 3 2016-05-08 10:50:55 PDT
Darin Adler
Comment 4 2016-05-08 10:51:30 PDT
OK, back to a nice small, contained patch again.
Darin Adler
Comment 5 2016-05-08 10:55:24 PDT
Nope, forgot to get rid of Optional!
Darin Adler
Comment 6 2016-05-08 10:56:11 PDT
Darin Adler
Comment 7 2016-05-08 10:56:29 PDT
OK, that’s better. Ready to review and land now, I think.
Chris Dumez
Comment 8 2016-05-08 11:10:50 PDT
Comment on attachment 278366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278366&action=review r=me > Source/WebCore/html/HTMLSlotElement.cpp:-138 > - bool bubbles = false; bubbles had the benefit of matching the DOM spec.
Darin Adler
Comment 9 2016-05-08 11:30:34 PDT
Comment on attachment 278366 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278366&action=review >> Source/WebCore/html/HTMLSlotElement.cpp:-138 >> - bool bubbles = false; > > bubbles had the benefit of matching the DOM spec. Very interesting. I changed this to match Event.idl and Event.h; didn’t realize they were out of sync with the spec.
Darin Adler
Comment 10 2016-05-08 11:34:39 PDT
Note You need to log in before you can comment on or make changes to this bug.