Bug 155424 – Add slotchange event

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Ryosuke Niwa]

Add the support for slotchange event as discussed in https://github.com/w3c/webcomponents/issues/288.

[Ryosuke Niwa]

<rdar://problem/24997534>

[Ryosuke Niwa]

Created attachment 273933 [details]
WIP

[Ryosuke Niwa]

Created attachment 273943 [details]
Implements the feature

[Ryosuke Niwa]

Created attachment 273944 [details]
Updated a change log

[Antti Koivisto]

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.

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

Created attachment 273950 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 273950 [details]
Patch for landing

Wait for EWS

[Ryosuke Niwa]

I don't think the failure of editing/pasteboard/paste-noplugin-xhtml.xhtml is related to this patch.

[WebKit Commit Bot]

Comment on attachment 273950 [details]
Patch for landing

Clearing flags on attachment: 273950

Committed r198115: <http://trac.webkit.org/changeset/198115>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.