Bug 155424 - Add slotchange event
Summary: Add slotchange event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 148695
  Show dependency treegraph
 
Reported: 2016-03-13 23:33 PDT by Ryosuke Niwa
Modified: 2016-03-14 05:02 PDT (History)
7 users (show)

See Also:


Attachments
WIP (19.97 KB, patch)
2016-03-14 00:32 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Implements the feature (53.73 KB, patch)
2016-03-14 01:56 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Updated a change log (53.83 KB, patch)
2016-03-14 02:01 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (53.91 KB, patch)
2016-03-14 03:14 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2016-03-13 23:33:20 PDT
Add the support for slotchange event as discussed in https://github.com/w3c/webcomponents/issues/288.
Comment 1 Ryosuke Niwa 2016-03-13 23:34:06 PDT
<rdar://problem/24997534>
Comment 2 Ryosuke Niwa 2016-03-14 00:32:39 PDT
Created attachment 273933 [details]
WIP
Comment 3 Ryosuke Niwa 2016-03-14 01:56:15 PDT
Created attachment 273943 [details]
Implements the feature
Comment 4 Ryosuke Niwa 2016-03-14 02:01:16 PDT
Created attachment 273944 [details]
Updated a change log
Comment 5 Antti Koivisto 2016-03-14 02:24:38 PDT
Comment on attachment 273944 [details]
Updated a change log

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

> Source/WebCore/dom/ShadowRoot.h:97
> +    void hostChildElementDidChangeSlotAttr(const AtomicString& oldValue, const AtomicString& newValue);

Attr -> Attribute

> Source/WebCore/dom/SlotAssignment.cpp:131
> +static void recursivelyFireSlotChangeEvent(HTMLSlotElement* slotElement)

HTMLSlotElement&

> Source/WebCore/dom/SlotAssignment.cpp:136
> +    if (auto* slotParent = slotElement->parentElement()) {
> +        if (auto* shadowRootOfSlotParent = slotParent->shadowRoot())

I don't understand this. Why not just slot elements shadow root? Real element parent of a slot shouldn't have any special status.

If this is valid please add comment explaining it.

This could use early returns.
Comment 6 Ryosuke Niwa 2016-03-14 03:01:40 PDT
Comment on attachment 273944 [details]
Updated a change log

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

Thanks for the review!

>> Source/WebCore/dom/ShadowRoot.h:97
>> +    void hostChildElementDidChangeSlotAttr(const AtomicString& oldValue, const AtomicString& newValue);
> 
> Attr -> Attribute

Fixed.

>> Source/WebCore/dom/SlotAssignment.cpp:131
>> +static void recursivelyFireSlotChangeEvent(HTMLSlotElement* slotElement)
> 
> HTMLSlotElement&

Fixed.

>> Source/WebCore/dom/SlotAssignment.cpp:136
>> +        if (auto* shadowRootOfSlotParent = slotParent->shadowRoot())
> 
> I don't understand this. Why not just slot elements shadow root? Real element parent of a slot shouldn't have any special status.
> 
> If this is valid please add comment explaining it.
> 
> This could use early returns.

This is the case in which slotElement's parent has a shadow root
and slotElement is assigned to a slot in that shadow root as we talked on IRC.

Fixed the code to early returns.
Comment 7 Ryosuke Niwa 2016-03-14 03:14:14 PDT
Created attachment 273950 [details]
Patch for landing
Comment 8 Ryosuke Niwa 2016-03-14 03:14:33 PDT
Comment on attachment 273950 [details]
Patch for landing

Wait for EWS
Comment 9 Ryosuke Niwa 2016-03-14 03:58:50 PDT
I don't think the failure of editing/pasteboard/paste-noplugin-xhtml.xhtml is related to this patch.
Comment 10 WebKit Commit Bot 2016-03-14 05:02:09 PDT
Comment on attachment 273950 [details]
Patch for landing

Clearing flags on attachment: 273950

Committed r198115: <http://trac.webkit.org/changeset/198115>
Comment 11 WebKit Commit Bot 2016-03-14 05:02:14 PDT
All reviewed patches have been landed.  Closing bug.