WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(7.12 KB, patch)
2016-05-08 10:50 PDT
,
Darin Adler
no flags
Details
Formatted Diff
Diff
Patch
(7.09 KB, patch)
2016-05-08 10:56 PDT
,
Darin Adler
cdumez
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Darin Adler
Comment 1
2016-05-07 23:03:16 PDT
Created
attachment 278353
[details]
Patch
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
Created
attachment 278365
[details]
Patch
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
Created
attachment 278366
[details]
Patch
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
Committed
r200557
: <
http://trac.webkit.org/changeset/200557
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug