Bug 155424

Summary: Add slotchange event
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cmarcelo, commit-queue, esprehn+autocc, gyuyoung.kim, kangil.han, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 148695    
Attachments:
Description Flags
WIP
none
Implements the feature
none
Updated a change log
none
Patch for landing none

Ryosuke Niwa
Reported 2016-03-13 23:33:20 PDT
Add the support for slotchange event as discussed in https://github.com/w3c/webcomponents/issues/288.
Attachments
WIP (19.97 KB, patch)
2016-03-14 00:32 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Implements the feature (53.73 KB, patch)
2016-03-14 01:56 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Updated a change log (53.83 KB, patch)
2016-03-14 02:01 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Patch for landing (53.91 KB, patch)
2016-03-14 03:14 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2016-03-13 23:34:06 PDT
<rdar://problem/24997534>
Ryosuke Niwa
Comment 2 2016-03-14 00:32:39 PDT
Created attachment 273933 [details] WIP
Ryosuke Niwa
Comment 3 2016-03-14 01:56:15 PDT
Created attachment 273943 [details] Implements the feature
Ryosuke Niwa
Comment 4 2016-03-14 02:01:16 PDT
Created attachment 273944 [details] Updated a change log
Antti Koivisto
Comment 5 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.
Ryosuke Niwa
Comment 6 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.
Ryosuke Niwa
Comment 7 2016-03-14 03:14:14 PDT
Created attachment 273950 [details] Patch for landing
Ryosuke Niwa
Comment 8 2016-03-14 03:14:33 PDT
Comment on attachment 273950 [details] Patch for landing Wait for EWS
Ryosuke Niwa
Comment 9 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.
WebKit Commit Bot
Comment 10 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>
WebKit Commit Bot
Comment 11 2016-03-14 05:02:14 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.