Title: [236002] trunk
Revision
236002
Author
[email protected]
Date
2018-09-13 22:56:19 -0700 (Thu, 13 Sep 2018)

Log Message

Capturing event listeners are called during bubbling phase for shadow hosts
https://bugs.webkit.org/show_bug.cgi?id=174288
LayoutTests/imported/w3c:


Reviewed by Darin Adler.

* web-platform-tests/dom/events/Event-dispatch-handlers-changed-expected.txt: Rebaselined. This test's
expectation is now wrong because event listner 3 is added after the event listener list is cloned for
capturing event listeners but before cloned for bubbling event listeners. As a result, event listener 3
is now invoked. It used to be not called because both bubbling and capturing event listeners are called
after cloning the event listner list once, which didn't have event listener 3.

* web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt: Rebaselined. This test expects
event listener 2, which is bubbling, to be called between two capturing event listeners 1 and 3, which
is no longer true after this patch.

Source/WebCore:

<rdar://problem/33530455>

Reviewed by Darin Adler.

Implemented the new behavior proposed in https://github.com/whatwg/dom/pull/686 [1] to fix the problem
that capturing event listeners on a shadow host is invoked during bubbling phase when an event is
dispatched within its shadow tree.

To see why this is a problem, suppose we fire a composed event at span#target in the following DOM tree:
  section#hostParent
    + div#host -- ShadowRoot
                    - p#parent
                        - span#target
Then capturing and bubbling event listeners on #target, #parent, #host, and #hostParent are invoked in
the following order in WebKit & Chrome right now:

1. #hostParent, capturing, eventPhase: CAPTURING_PHASE
2. #parent, capturing, eventPhase: CAPTURING_PHASE
3. #target, capturing, eventPhase: AT_TARGET
4. #target, non-capturing, eventPhase: AT_TARGET
5. #parent, non-capturing, eventPhase: BUBBLING_PHASE
6. #host, capturing, eventPhase: AT_TARGET
7. #host, non-capturing, eventPhase: AT_TARGET
8. #hostParent, non-capturing, eventPhase: BUBBLING_PHASE

This is counter-intuitive because capturing event listeners on #host isn't invoked until bubblign phase
started. A more natural ordering would be:

1. #hostParent, capturing, eventPhase: CAPTURING_PHASE
2. #host, capturing, eventPhase: AT_TARGET
3. #parent, capturing, eventPhase: CAPTURING_PHASE
4. #target, capturing, eventPhase: AT_TARGET
5. #target, non-capturing, eventPhase: AT_TARGET
6. #parent, non-capturing, eventPhase: BUBBLING_PHASE
7. #host, non-capturing, eventPhase: AT_TARGET
8. #hostParent, non-capturing, eventPhase: BUBBLING_PHASE

This also happens to be the order by which Gecko's current shadow DOM implementation invoke event listners.
This patch implements this new behavior using the spec-change proposed in [1]. Note that this patch also
impacts the invocation order of event listeners when there is no shadow tree. Namely, before this patch,
event listeners on the event's target is invoked in the registration order. After this patch, all capturing
event listeners are invoked before bubbling event listeners are invoked.

To implement this behavior, this patch introduces EventTarget::EventInvokePhase indicating whether we're
in the capturing phase or bubbling phase to EventTarget::fireEventListeners. We can't use Event's eventPhase
enum because that's set to Event::Phase::AT_TARGET when we're at a shadow host.

Test: fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html

* Modules/modern-media-controls/media/media-controller-support.js:
(MediaControllerSupport.prototype.enable): Use capturing event listeners so that we can update the states of
media controls before author scripts recieve the event.
(MediaControllerSupport.prototype.disable): Ditto.
* dom/EventContext.cpp:
(WebCore::EventContext::handleLocalEvents const):
(WebCore::MouseOrFocusEventContext::handleLocalEvents const):
(WebCore::TouchEventContext::handleLocalEvents const):
* dom/EventContext.h:
* dom/EventDispatcher.cpp:
(WebCore::dispatchEventInDOM): Invoke capturing event listners even when target and current target are same.
This happens when the current target is a shadow host and event's target is in its shadow tree. Also merged
the special code path for the event's target with the code in the bubbling phase.
* dom/EventPath.cpp:
(WebCore::WindowEventContext::handleLocalEvents const):
* dom/EventTarget.cpp:
(WebCore::EventTarget::dispatchEvent): Invoke capturing and bubbling event listeners in the order.
(WebCore::EventTarget::fireEventListeners):
(WebCore::EventTarget::innerInvokeEventListeners): Renamed from fireEventListeners to match the spec. Use
EventInvokePhase to filter out event listeners so that we can invoke capturing event listners before bubbling
event listeners even when eventPhase is Event::Phase::AT_TARGET.
* dom/EventTarget.h:
* dom/Node.cpp:
(WebCore::Node::handleLocalEvents):
* dom/Node.h:
* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::handleLocalEvents):
* html/HTMLFormElement.h:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::dispatchEvent):

LayoutTests:


Reviewed by Darin Adler.

Added a W3C style testharness.js test and rebaselined two tests. See below for rationals of rebaselines.

* fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees-expected.txt: Added.
* fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html: Added.

* media/media-load-event-expected.txt: Rebaselined. The logging of oncanplaythrough event is now happening
before canplaythrough() is called because the logging is done by waitForEvent which uses a capturing event
listener whereas canplaythrough is called by a event handler, which is non-capturing.

* platform/ios-11/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt:
* platform/ios/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (236001 => 236002)


--- trunk/LayoutTests/ChangeLog	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/ChangeLog	2018-09-14 05:56:19 UTC (rev 236002)
@@ -1,3 +1,22 @@
+2018-09-13  Ryosuke Niwa  <[email protected]>
+
+        Capturing event listeners are called during bubbling phase for shadow hosts
+        https://bugs.webkit.org/show_bug.cgi?id=174288
+
+        Reviewed by Darin Adler.
+
+        Added a W3C style testharness.js test and rebaselined two tests. See below for rationals of rebaselines.
+
+        * fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees-expected.txt: Added.
+        * fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html: Added.
+
+        * media/media-load-event-expected.txt: Rebaselined. The logging of oncanplaythrough event is now happening
+        before canplaythrough() is called because the logging is done by waitForEvent which uses a capturing event
+        listener whereas canplaythrough is called by a event handler, which is non-capturing.
+
+        * platform/ios-11/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt:
+        * platform/ios/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt:
+
 2018-09-13  Justin Fan  <[email protected]>
 
         Update webkit-webgl-test-harness.js for more details on WebGL 2 conformance tests part 4

Added: trunk/LayoutTests/fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees-expected.txt (0 => 236002)


--- trunk/LayoutTests/fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees-expected.txt	2018-09-14 05:56:19 UTC (rev 236002)
@@ -0,0 +1,7 @@
+
+PASS Capturing event listeners should be invoked before bubbling event listeners on the target without shadow trees 
+PASS Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a shadow tree 
+PASS Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a doubly nested shadow tree 
+PASS Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched via a slot 
+PASS Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a shadow tree which passes through another shadow tree 
+

Added: trunk/LayoutTests/fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html (0 => 236002)


--- trunk/LayoutTests/fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html	                        (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html	2018-09-14 05:56:19 UTC (rev 236002)
@@ -0,0 +1,191 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Shadow DOM: Capturing event listeners should be invoked before bubbling event listeners</title>
+<meta name="author" title="Ryosuke Niwa" href=""
+<script src=""
+<script src=""
+<script src=""
+<script>
+
+function attachEventListeners(eventType, tree) {
+    const eventLogs = [];
+    const makeComposedPathResult = (event) => event.composedPath().map((node) => node.id)
+    for (const id in tree) {
+        const node = tree[id];
+        node.addEventListener(eventType, event => eventLogs.push(
+            ['bubbling', event.eventPhase, event.target.id, event.currentTarget.id, makeComposedPathResult(event)]), {capture: false});
+        node.addEventListener(eventType, event => eventLogs.push(
+            ['capturing', event.eventPhase, event.target.id, event.currentTarget.id, makeComposedPathResult(event)]), {capture: true});
+    }
+    return eventLogs;
+}
+
+</script>
+</head>
+<body>
+
+<div id="test1">
+    <div id="parent">
+        <div id="target"></div>
+    </div>
+</div>
+<script>
+test(() => {
+    const tree = createTestTree(document.getElementById('test1'));
+    const logs = attachEventListeners('my-event', tree);
+    tree.target.dispatchEvent(new Event('my-event', { bubbles: true, composed: true }));
+
+    const composedPath = ['target', 'parent', 'test1'];
+    assert_object_equals(logs, [
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'test1', composedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'parent', composedPath],
+        ['capturing', Event.AT_TARGET, 'target', 'target', composedPath],
+        ['bubbling', Event.AT_TARGET, 'target', 'target', composedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'parent', composedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'test1', composedPath],
+    ]);
+}, 'Capturing event listeners should be invoked before bubbling event listeners on the target without shadow trees');
+</script>
+
+<div id="test2">
+    <div id="host">
+        <template id="shadowRoot" data-mode="closed">
+            <div id="target"></div>
+        </template>
+    </div>
+</div>
+<script>
+test(() => {
+    const tree = createTestTree(document.getElementById('test2'));
+    const logs = attachEventListeners('my-event', tree);
+    tree.target.dispatchEvent(new Event('my-event', { bubbles: true, composed: true }));
+
+    const innerComposedPath = ['target', 'shadowRoot', 'host', 'test2'];
+    const outerComposedPath = ['host', 'test2'];
+    assert_object_equals(logs, [
+        ['capturing', Event.CAPTURING_PHASE, 'host', 'test2', outerComposedPath],
+        ['capturing', Event.AT_TARGET, 'host', 'host', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'shadowRoot', innerComposedPath],
+        ['capturing', Event.AT_TARGET, 'target', 'target', innerComposedPath],
+        ['bubbling', Event.AT_TARGET, 'target', 'target', innerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'shadowRoot', innerComposedPath],
+        ['bubbling', Event.AT_TARGET, 'host', 'host', outerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'host', 'test2', outerComposedPath],
+    ]);
+}, 'Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a shadow tree');
+</script>
+
+<div id="test3">
+    <div id="outerHost">
+        <template id="outerShadowRoot" data-mode="closed">
+            <div id="innerHost">
+                <template id="innerShadowRoot" data-mode="closed">
+                    <div id="target"></div>
+                </template>
+            </div>
+        </template>
+    </div>
+</div>
+<script>
+test(() => {
+    const tree = createTestTree(document.getElementById('test3'));
+    const logs = attachEventListeners('my-event', tree);
+    tree.target.dispatchEvent(new Event('my-event', { bubbles: true, composed: true }));
+
+    const innerShadowComposedPath = ['target', 'innerShadowRoot', 'innerHost', 'outerShadowRoot', 'outerHost', 'test3'];
+    const outerShadowComposedPath = ['innerHost', 'outerShadowRoot', 'outerHost', 'test3'];
+    const outerComposedPath = ['outerHost', 'test3'];
+    assert_object_equals(logs, [
+        ['capturing', Event.CAPTURING_PHASE, 'outerHost', 'test3', outerComposedPath],
+        ['capturing', Event.AT_TARGET, 'outerHost', 'outerHost', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'innerHost', 'outerShadowRoot', outerShadowComposedPath],
+        ['capturing', Event.AT_TARGET, 'innerHost', 'innerHost', outerShadowComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'innerShadowRoot', innerShadowComposedPath],
+        ['capturing', Event.AT_TARGET, 'target', 'target', innerShadowComposedPath],
+
+        ['bubbling', Event.AT_TARGET, 'target', 'target', innerShadowComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'innerShadowRoot', innerShadowComposedPath],
+        ['bubbling', Event.AT_TARGET, 'innerHost', 'innerHost', outerShadowComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'innerHost', 'outerShadowRoot', outerShadowComposedPath],
+        ['bubbling', Event.AT_TARGET, 'outerHost', 'outerHost', outerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'outerHost', 'test3', outerComposedPath],
+    ]);
+}, 'Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a doubly nested shadow tree');
+</script>
+
+<div id="test4">
+    <div id="host">
+        <template id="shadowRoot" data-mode="closed">
+            <slot id="slot"></slot>
+        </template>
+        <div id="target"></div>
+    </div>
+</div>
+<script>
+test(() => {
+    const tree = createTestTree(document.getElementById('test4'));
+    const logs = attachEventListeners('my-event', tree);
+    tree.target.dispatchEvent(new Event('my-event', { bubbles: true, composed: true }));
+
+    const innerComposedPath = ['target', 'slot', 'shadowRoot', 'host', 'test4'];
+    const outerComposedPath = ['target', 'host', 'test4'];
+    assert_object_equals(logs, [
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'test4', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'host', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'shadowRoot', innerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'slot', innerComposedPath],
+        ['capturing', Event.AT_TARGET, 'target', 'target', outerComposedPath],
+
+        ['bubbling', Event.AT_TARGET, 'target', 'target', outerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'slot', innerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'shadowRoot', innerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'host', outerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'test4', outerComposedPath],
+    ]);
+}, 'Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched via a slot');
+</script>
+
+<div id="test5">
+    <div id="upperHost">
+        <template id="upperShadowRoot" data-mode="closed">
+            <slot id="upperSlot"></slot>
+        </template>
+        <div id="lowerHost">
+            <template id="lowerShadowRoot" data-mode="closed">
+                <div id="target"></div>
+            </template>
+        </div>
+    </div>
+</div>
+<script>
+test(() => {
+    const tree = createTestTree(document.getElementById('test5'));
+    const logs = attachEventListeners('my-event', tree);
+    tree.target.dispatchEvent(new Event('my-event', { bubbles: true, composed: true }));
+
+    const lowerComposedPath = ['target', 'lowerShadowRoot', 'lowerHost', 'upperHost', 'test5'];
+    const upperComposedPath = ['lowerHost', 'upperSlot', 'upperShadowRoot', 'upperHost', 'test5'];
+    const outerComposedPath = ['lowerHost', 'upperHost', 'test5'];
+    assert_object_equals(logs, [
+        ['capturing', Event.CAPTURING_PHASE, 'lowerHost', 'test5', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'lowerHost', 'upperHost', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'lowerHost', 'upperShadowRoot', upperComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'lowerHost', 'upperSlot', upperComposedPath],
+        ['capturing', Event.AT_TARGET, 'lowerHost', 'lowerHost', outerComposedPath],
+        ['capturing', Event.CAPTURING_PHASE, 'target', 'lowerShadowRoot', lowerComposedPath],
+        ['capturing', Event.AT_TARGET, 'target', 'target', lowerComposedPath],
+
+        ['bubbling', Event.AT_TARGET, 'target', 'target', lowerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'target', 'lowerShadowRoot', lowerComposedPath],
+        ['bubbling', Event.AT_TARGET, 'lowerHost', 'lowerHost', outerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'lowerHost', 'upperSlot', upperComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'lowerHost', 'upperShadowRoot', upperComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'lowerHost', 'upperHost', outerComposedPath],
+        ['bubbling', Event.BUBBLING_PHASE, 'lowerHost', 'test5', outerComposedPath],
+    ]);
+}, 'Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a shadow tree which passes through another shadow tree');
+</script>
+
+</body>
+</html>

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (236001 => 236002)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2018-09-14 05:56:19 UTC (rev 236002)
@@ -1,3 +1,20 @@
+2018-09-12  Ryosuke Niwa  <[email protected]>
+
+        Capturing event listeners are called during bubbling phase for shadow hosts
+        https://bugs.webkit.org/show_bug.cgi?id=174288
+
+        Reviewed by Darin Adler.
+
+        * web-platform-tests/dom/events/Event-dispatch-handlers-changed-expected.txt: Rebaselined. This test's
+        expectation is now wrong because event listner 3 is added after the event listener list is cloned for
+        capturing event listeners but before cloned for bubbling event listeners. As a result, event listener 3
+        is now invoked. It used to be not called because both bubbling and capturing event listeners are called
+        after cloning the event listner list once, which didn't have event listener 3.
+
+        * web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt: Rebaselined. This test expects
+        event listener 2, which is bubbling, to be called between two capturing event listeners 1 and 3, which
+        is no longer true after this patch.
+
 2018-09-12  Chris Dumez  <[email protected]>
 
         Unreviewed, rebaseline imported/w3c/web-platform-tests/url/failure.html after r235808.

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/Event-dispatch-handlers-changed-expected.txt (236001 => 236002)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/Event-dispatch-handlers-changed-expected.txt	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/Event-dispatch-handlers-changed-expected.txt	2018-09-14 05:56:19 UTC (rev 236002)
@@ -1,3 +1,3 @@
 
-PASS  Dispatch additional events inside an event listener  
+FAIL  Dispatch additional events inside an event listener  assert_array_equals: actual_targets lengths differ, expected 16 got 17
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt (236001 => 236002)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt	2018-09-14 05:56:19 UTC (rev 236002)
@@ -25,5 +25,5 @@
 PASS If the event's dispatch flag is set, an InvalidStateError must be thrown. 
 PASS Exceptions from event listeners must not be propagated. 
 PASS Event listeners added during dispatch should be called 
-PASS Event listeners should be called in order of addition 
+FAIL Event listeners should be called in order of addition assert_array_equals: property 1, expected 2 but got 3
 

Modified: trunk/LayoutTests/media/media-load-event-expected.txt (236001 => 236002)


--- trunk/LayoutTests/media/media-load-event-expected.txt	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/media/media-load-event-expected.txt	2018-09-14 05:56:19 UTC (rev 236002)
@@ -8,11 +8,11 @@
 EVENT(durationchange)
 EVENT(loadeddata)
 EVENT(canplaythrough)
+EVENT(canplaythrough)
 
 RUN(document.getElementById('parent').appendChild(mediaElement))
 RUN(mediaElement.play())
 
-EVENT(canplaythrough)
 EVENT(play)
 EVENT(playing)
 

Modified: trunk/LayoutTests/platform/ios/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt (236001 => 236002)


--- trunk/LayoutTests/platform/ios/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/platform/ios/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt	2018-09-14 05:56:19 UTC (rev 236002)
@@ -25,5 +25,5 @@
 PASS If the event's dispatch flag is set, an InvalidStateError must be thrown. 
 PASS Exceptions from event listeners must not be propagated. 
 PASS Event listeners added during dispatch should be called 
-PASS Event listeners should be called in order of addition 
+PASS Event listeners should be +FAIL Event listeners should be called in order of addition assert_array_equals: property 1, expected 2 but got 3
 

Modified: trunk/LayoutTests/platform/ios-11/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt (236001 => 236002)


--- trunk/LayoutTests/platform/ios-11/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/LayoutTests/platform/ios-11/imported/w3c/web-platform-tests/dom/events/EventTarget-dispatchEvent-expected.txt	2018-09-14 05:56:19 UTC (rev 236002)
@@ -25,5 +25,5 @@
 PASS If the event's dispatch flag is set, an InvalidStateError must be thrown. 
 PASS Exceptions from event listeners must not be propagated. 
 PASS Event listeners added during dispatch should be called 
-PASS Event listeners should be called in order of addition 
+FAIL Event listeners should be called in order of addition assert_array_equals: property 1, expected 2 but got 3
 

Modified: trunk/Source/WebCore/ChangeLog (236001 => 236002)


--- trunk/Source/WebCore/ChangeLog	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/ChangeLog	2018-09-14 05:56:19 UTC (rev 236002)
@@ -1,3 +1,87 @@
+2018-09-13  Ryosuke Niwa  <[email protected]>
+
+        Capturing event listeners are called during bubbling phase for shadow hosts
+        https://bugs.webkit.org/show_bug.cgi?id=174288
+        <rdar://problem/33530455>
+
+        Reviewed by Darin Adler.
+
+        Implemented the new behavior proposed in https://github.com/whatwg/dom/pull/686 [1] to fix the problem
+        that capturing event listeners on a shadow host is invoked during bubbling phase when an event is
+        dispatched within its shadow tree.
+
+        To see why this is a problem, suppose we fire a composed event at span#target in the following DOM tree:
+          section#hostParent
+            + div#host -- ShadowRoot
+                            - p#parent
+                                - span#target
+        Then capturing and bubbling event listeners on #target, #parent, #host, and #hostParent are invoked in
+        the following order in WebKit & Chrome right now:
+
+        1. #hostParent, capturing, eventPhase: CAPTURING_PHASE
+        2. #parent, capturing, eventPhase: CAPTURING_PHASE
+        3. #target, capturing, eventPhase: AT_TARGET
+        4. #target, non-capturing, eventPhase: AT_TARGET
+        5. #parent, non-capturing, eventPhase: BUBBLING_PHASE
+        6. #host, capturing, eventPhase: AT_TARGET
+        7. #host, non-capturing, eventPhase: AT_TARGET
+        8. #hostParent, non-capturing, eventPhase: BUBBLING_PHASE
+
+        This is counter-intuitive because capturing event listeners on #host isn't invoked until bubblign phase
+        started. A more natural ordering would be:
+
+        1. #hostParent, capturing, eventPhase: CAPTURING_PHASE
+        2. #host, capturing, eventPhase: AT_TARGET
+        3. #parent, capturing, eventPhase: CAPTURING_PHASE
+        4. #target, capturing, eventPhase: AT_TARGET
+        5. #target, non-capturing, eventPhase: AT_TARGET
+        6. #parent, non-capturing, eventPhase: BUBBLING_PHASE
+        7. #host, non-capturing, eventPhase: AT_TARGET
+        8. #hostParent, non-capturing, eventPhase: BUBBLING_PHASE
+
+        This also happens to be the order by which Gecko's current shadow DOM implementation invoke event listners.
+        This patch implements this new behavior using the spec-change proposed in [1]. Note that this patch also
+        impacts the invocation order of event listeners when there is no shadow tree. Namely, before this patch,
+        event listeners on the event's target is invoked in the registration order. After this patch, all capturing
+        event listeners are invoked before bubbling event listeners are invoked.
+
+        To implement this behavior, this patch introduces EventTarget::EventInvokePhase indicating whether we're
+        in the capturing phase or bubbling phase to EventTarget::fireEventListeners. We can't use Event's eventPhase
+        enum because that's set to Event::Phase::AT_TARGET when we're at a shadow host.
+
+        Test: fast/shadow-dom/capturing-and-bubbling-event-listeners-across-shadow-trees.html
+
+        * Modules/modern-media-controls/media/media-controller-support.js:
+        (MediaControllerSupport.prototype.enable): Use capturing event listeners so that we can update the states of
+        media controls before author scripts recieve the event.
+        (MediaControllerSupport.prototype.disable): Ditto.
+        * dom/EventContext.cpp:
+        (WebCore::EventContext::handleLocalEvents const):
+        (WebCore::MouseOrFocusEventContext::handleLocalEvents const):
+        (WebCore::TouchEventContext::handleLocalEvents const):
+        * dom/EventContext.h:
+        * dom/EventDispatcher.cpp:
+        (WebCore::dispatchEventInDOM): Invoke capturing event listners even when target and current target are same.
+        This happens when the current target is a shadow host and event's target is in its shadow tree. Also merged
+        the special code path for the event's target with the code in the bubbling phase.
+        * dom/EventPath.cpp:
+        (WebCore::WindowEventContext::handleLocalEvents const):
+        * dom/EventTarget.cpp:
+        (WebCore::EventTarget::dispatchEvent): Invoke capturing and bubbling event listeners in the order.
+        (WebCore::EventTarget::fireEventListeners):
+        (WebCore::EventTarget::innerInvokeEventListeners): Renamed from fireEventListeners to match the spec. Use
+        EventInvokePhase to filter out event listeners so that we can invoke capturing event listners before bubbling
+        event listeners even when eventPhase is Event::Phase::AT_TARGET.
+        * dom/EventTarget.h:
+        * dom/Node.cpp:
+        (WebCore::Node::handleLocalEvents):
+        * dom/Node.h:
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::handleLocalEvents):
+        * html/HTMLFormElement.h:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::dispatchEvent):
+
 2018-09-13  Megan Gardner  <[email protected]>
 
         Fix color stop blending in conic gradients for stops past 1

Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js (236001 => 236002)


--- trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js	2018-09-14 05:56:19 UTC (rev 236002)
@@ -38,7 +38,7 @@
     enable()
     {
         for (let eventType of this.mediaEvents)
-            this.mediaController.media.addEventListener(eventType, this);
+            this.mediaController.media.addEventListener(eventType, this, true);
 
         for (let tracks of this.tracksToMonitor) {
             for (let eventType of ["change", "addtrack", "removetrack"])
@@ -55,7 +55,7 @@
     disable()
     {
         for (let eventType of this.mediaEvents)
-            this.mediaController.media.removeEventListener(eventType, this);
+            this.mediaController.media.removeEventListener(eventType, this, true);
 
         for (let tracks of this.tracksToMonitor) {
             for (let eventType of ["change", "addtrack", "removetrack"])

Modified: trunk/Source/WebCore/dom/EventContext.cpp (236001 => 236002)


--- trunk/Source/WebCore/dom/EventContext.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/EventContext.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -45,15 +45,15 @@
 
 EventContext::~EventContext() = default;
 
-void EventContext::handleLocalEvents(Event& event) const
+void EventContext::handleLocalEvents(Event& event, EventInvokePhase phase) const
 {
     event.setTarget(m_target.get());
     event.setCurrentTarget(m_currentTarget.get());
     // FIXME: Consider merging handleLocalEvents and fireEventListeners.
     if (m_node)
-        m_node->handleLocalEvents(event);
+        m_node->handleLocalEvents(event, phase);
     else
-        m_currentTarget->fireEventListeners(event);
+        m_currentTarget->fireEventListeners(event, phase);
 }
 
 bool EventContext::isMouseOrFocusEventContext() const
@@ -73,11 +73,11 @@
 
 MouseOrFocusEventContext::~MouseOrFocusEventContext() = default;
 
-void MouseOrFocusEventContext::handleLocalEvents(Event& event) const
+void MouseOrFocusEventContext::handleLocalEvents(Event& event, EventInvokePhase phase) const
 {
     if (m_relatedTarget)
         event.setRelatedTarget(*m_relatedTarget);
-    EventContext::handleLocalEvents(event);
+    EventContext::handleLocalEvents(event, phase);
 }
 
 bool MouseOrFocusEventContext::isMouseOrFocusEventContext() const
@@ -97,7 +97,7 @@
 
 TouchEventContext::~TouchEventContext() = default;
 
-void TouchEventContext::handleLocalEvents(Event& event) const
+void TouchEventContext::handleLocalEvents(Event& event, EventInvokePhase phase) const
 {
     checkReachability(m_touches);
     checkReachability(m_targetTouches);
@@ -106,7 +106,7 @@
     touchEvent.setTouches(m_touches.ptr());
     touchEvent.setTargetTouches(m_targetTouches.ptr());
     touchEvent.setChangedTouches(m_changedTouches.ptr());
-    EventContext::handleLocalEvents(event);
+    EventContext::handleLocalEvents(event, phase);
 }
 
 bool TouchEventContext::isTouchEventContext() const

Modified: trunk/Source/WebCore/dom/EventContext.h (236001 => 236002)


--- trunk/Source/WebCore/dom/EventContext.h	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/EventContext.h	2018-09-14 05:56:19 UTC (rev 236002)
@@ -36,6 +36,8 @@
 class EventContext {
     WTF_MAKE_FAST_ALLOCATED;
 public:
+    using EventInvokePhase = EventTarget::EventInvokePhase;
+
     EventContext(Node*, EventTarget* currentTarget, EventTarget*);
     virtual ~EventContext();
 
@@ -43,7 +45,7 @@
     EventTarget* currentTarget() const { return m_currentTarget.get(); }
     EventTarget* target() const { return m_target.get(); }
 
-    virtual void handleLocalEvents(Event&) const;
+    virtual void handleLocalEvents(Event&, EventInvokePhase) const;
 
     virtual bool isMouseOrFocusEventContext() const;
     virtual bool isTouchEventContext() const;
@@ -67,7 +69,7 @@
     void setRelatedTarget(Node*);
 
 private:
-    void handleLocalEvents(Event&) const final;
+    void handleLocalEvents(Event&, EventInvokePhase) const final;
     bool isMouseOrFocusEventContext() const final;
 
     RefPtr<Node> m_relatedTarget;
@@ -84,7 +86,7 @@
     TouchList& touchList(TouchListType);
 
 private:
-    void handleLocalEvents(Event&) const final;
+    void handleLocalEvents(Event&, EventInvokePhase) const final;
     bool isTouchEventContext() const final;
 
     void checkReachability(const Ref<TouchList>&) const;

Modified: trunk/Source/WebCore/dom/EventDispatcher.cpp (236001 => 236002)


--- trunk/Source/WebCore/dom/EventDispatcher.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/EventDispatcher.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -75,26 +75,21 @@
 
 static void dispatchEventInDOM(Event& event, const EventPath& path)
 {
-    // Trigger capturing event handlers, starting at the top and working our way down.
-    event.setEventPhase(Event::CAPTURING_PHASE);
-
-    for (size_t i = path.size() - 1; i > 0; --i) {
-        const EventContext& eventContext = path.contextAt(i);
+    // Invoke capturing event listeners in the reverse order.
+    for (size_t i = path.size(); i > 0; --i) {
+        const EventContext& eventContext = path.contextAt(i - 1);
         if (eventContext.currentTarget() == eventContext.target())
-            continue;
-        eventContext.handleLocalEvents(event);
+            event.setEventPhase(Event::AT_TARGET);
+        else
+            event.setEventPhase(Event::CAPTURING_PHASE);
+        eventContext.handleLocalEvents(event, EventTarget::EventInvokePhase::Capturing);
         if (event.propagationStopped())
             return;
     }
 
-    event.setEventPhase(Event::AT_TARGET);
-    path.contextAt(0).handleLocalEvents(event);
-    if (event.propagationStopped())
-        return;
-
-    // Trigger bubbling event handlers, starting at the bottom and working our way up.
+    // Invoke bubbling event listeners.
     size_t size = path.size();
-    for (size_t i = 1; i < size; ++i) {
+    for (size_t i = 0; i < size; ++i) {
         const EventContext& eventContext = path.contextAt(i);
         if (eventContext.currentTarget() == eventContext.target())
             event.setEventPhase(Event::AT_TARGET);
@@ -102,7 +97,7 @@
             event.setEventPhase(Event::BUBBLING_PHASE);
         else
             continue;
-        eventContext.handleLocalEvents(event);
+        eventContext.handleLocalEvents(event, EventTarget::EventInvokePhase::Bubbling);
         if (event.propagationStopped())
             return;
     }

Modified: trunk/Source/WebCore/dom/EventPath.cpp (236001 => 236002)


--- trunk/Source/WebCore/dom/EventPath.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/EventPath.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -37,7 +37,7 @@
 public:
     WindowEventContext(Node&, DOMWindow&, EventTarget&);
 private:
-    void handleLocalEvents(Event&) const final;
+    void handleLocalEvents(Event&, EventInvokePhase) const final;
 };
 
 inline WindowEventContext::WindowEventContext(Node& node, DOMWindow& currentTarget, EventTarget& target)
@@ -45,11 +45,11 @@
 {
 }
 
-void WindowEventContext::handleLocalEvents(Event& event) const
+void WindowEventContext::handleLocalEvents(Event& event, EventInvokePhase phase) const
 {
     event.setTarget(m_target.get());
     event.setCurrentTarget(m_currentTarget.get());
-    m_currentTarget->fireEventListeners(event);
+    m_currentTarget->fireEventListeners(event, phase);
 }
 
 static inline bool shouldEventCrossShadowBoundary(Event& event, ShadowRoot& shadowRoot, EventTarget& target)

Modified: trunk/Source/WebCore/dom/EventTarget.cpp (236001 => 236002)


--- trunk/Source/WebCore/dom/EventTarget.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/EventTarget.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -183,6 +183,7 @@
 
 void EventTarget::dispatchEvent(Event& event)
 {
+    // FIXME: We should always use EventDispatcher.
     ASSERT(event.isInitialized());
     ASSERT(!event.isBeingDispatched());
 
@@ -190,7 +191,8 @@
     event.setCurrentTarget(this);
     event.setEventPhase(Event::AT_TARGET);
     event.resetBeforeDispatch();
-    fireEventListeners(event);
+    fireEventListeners(event, EventInvokePhase::Capturing);
+    fireEventListeners(event, EventInvokePhase::Bubbling);
     event.resetAfterDispatch();
 }
 
@@ -219,7 +221,8 @@
     return nullAtom();
 }
 
-void EventTarget::fireEventListeners(Event& event)
+// https://dom.spec.whatwg.org/#concept-event-listener-invoke
+void EventTarget::fireEventListeners(Event& event, EventInvokePhase phase)
 {
     ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::isEventAllowedInMainThread());
     ASSERT(event.isInitialized());
@@ -231,7 +234,7 @@
     SetForScope<bool> firingEventListenersScope(data->isFiringEventListeners, true);
 
     if (auto* listenersVector = data->eventListenerMap.find(event.type())) {
-        fireEventListeners(event, *listenersVector);
+        innerInvokeEventListeners(event, *listenersVector, phase);
         return;
     }
 
@@ -244,7 +247,7 @@
         if (auto* legacyListenersVector = data->eventListenerMap.find(legacyTypeName)) {
             AtomicString typeName = event.type();
             event.setType(legacyTypeName);
-            fireEventListeners(event, *legacyListenersVector);
+            innerInvokeEventListeners(event, *legacyListenersVector, phase);
             event.setType(typeName);
         }
     }
@@ -252,7 +255,8 @@
 
 // Intentionally creates a copy of the listeners vector to avoid event listeners added after this point from being run.
 // Note that removal still has an effect due to the removed field in RegisteredEventListener.
-void EventTarget::fireEventListeners(Event& event, EventListenerVector listeners)
+// https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
+void EventTarget::innerInvokeEventListeners(Event& event, EventListenerVector listeners, EventInvokePhase phase)
 {
     Ref<EventTarget> protectedThis(*this);
     ASSERT(!listeners.isEmpty());
@@ -268,9 +272,9 @@
         if (UNLIKELY(registeredListener->wasRemoved()))
             continue;
 
-        if (event.eventPhase() == Event::CAPTURING_PHASE && !registeredListener->useCapture())
+        if (phase == EventInvokePhase::Capturing && !registeredListener->useCapture())
             continue;
-        if (event.eventPhase() == Event::BUBBLING_PHASE && registeredListener->useCapture())
+        if (phase == EventInvokePhase::Bubbling && registeredListener->useCapture())
             continue;
 
         if (InspectorInstrumentation::isEventListenerDisabled(*this, event.type(), registeredListener->callback(), registeredListener->useCapture()))

Modified: trunk/Source/WebCore/dom/EventTarget.h (236001 => 236002)


--- trunk/Source/WebCore/dom/EventTarget.h	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/EventTarget.h	2018-09-14 05:56:19 UTC (rev 236002)
@@ -102,7 +102,8 @@
     bool hasActiveEventListeners(const AtomicString& eventType) const;
     const EventListenerVector& eventListeners(const AtomicString& eventType);
 
-    void fireEventListeners(Event&);
+    enum class EventInvokePhase { Capturing, Bubbling };
+    void fireEventListeners(Event&, EventInvokePhase);
     bool isFiringEventListeners() const;
 
     void visitJSEventListeners(JSC::SlotVisitor&);
@@ -120,7 +121,7 @@
     virtual void refEventTarget() = 0;
     virtual void derefEventTarget() = 0;
     
-    void fireEventListeners(Event&, EventListenerVector);
+    void innerInvokeEventListeners(Event&, EventListenerVector, EventInvokePhase);
 
     friend class EventListenerIterator;
 };

Modified: trunk/Source/WebCore/dom/Node.cpp (236001 => 236002)


--- trunk/Source/WebCore/dom/Node.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/Node.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -2307,7 +2307,7 @@
     }
 }
 
-void Node::handleLocalEvents(Event& event)
+void Node::handleLocalEvents(Event& event, EventInvokePhase phase)
 {
     if (!hasEventTargetData())
         return;
@@ -2316,7 +2316,7 @@
     if (is<Element>(*this) && downcast<Element>(*this).isDisabledFormControl() && event.isMouseEvent() && !event.isWheelEvent())
         return;
 
-    fireEventListeners(event);
+    fireEventListeners(event, phase);
 }
 
 void Node::dispatchScopedEvent(Event& event)

Modified: trunk/Source/WebCore/dom/Node.h (236001 => 236002)


--- trunk/Source/WebCore/dom/Node.h	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/dom/Node.h	2018-09-14 05:56:19 UTC (rev 236002)
@@ -486,7 +486,7 @@
 
     void dispatchScopedEvent(Event&);
 
-    virtual void handleLocalEvents(Event&);
+    virtual void handleLocalEvents(Event&, EventInvokePhase);
 
     void dispatchSubtreeModifiedEvent();
     void dispatchDOMActivateEvent(Event& underlyingClickEvent);

Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (236001 => 236002)


--- trunk/Source/WebCore/html/HTMLFormElement.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -142,13 +142,13 @@
     HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
 }
 
-void HTMLFormElement::handleLocalEvents(Event& event)
+void HTMLFormElement::handleLocalEvents(Event& event, EventInvokePhase phase)
 {
     if (event.eventPhase() != Event::CAPTURING_PHASE && is<Node>(event.target()) && event.target() != this && (event.type() == eventNames().submitEvent || event.type() == eventNames().resetEvent)) {
         event.stopPropagation();
         return;
     }
-    HTMLElement::handleLocalEvents(event);
+    HTMLElement::handleLocalEvents(event, phase);
 }
 
 unsigned HTMLFormElement::length() const

Modified: trunk/Source/WebCore/html/HTMLFormElement.h (236001 => 236002)


--- trunk/Source/WebCore/html/HTMLFormElement.h	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/html/HTMLFormElement.h	2018-09-14 05:56:19 UTC (rev 236002)
@@ -133,7 +133,7 @@
     void removedFromAncestor(RemovalType, ContainerNode&) final;
     void finishParsingChildren() final;
 
-    void handleLocalEvents(Event&) final;
+    void handleLocalEvents(Event&, EventInvokePhase) final;
 
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
     bool isURLAttribute(const Attribute&) const final;

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (236001 => 236002)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2018-09-14 02:19:45 UTC (rev 236001)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2018-09-14 05:56:19 UTC (rev 236002)
@@ -2052,7 +2052,9 @@
     event.setEventPhase(Event::AT_TARGET);
     event.resetBeforeDispatch();
     auto cookie = InspectorInstrumentation::willDispatchEventOnWindow(frame(), event, *this);
-    fireEventListeners(event);
+    // FIXME: We should use EventDispatcher everywhere.
+    fireEventListeners(event, EventInvokePhase::Capturing);
+    fireEventListeners(event, EventInvokePhase::Bubbling);
     InspectorInstrumentation::didDispatchEventOnWindow(cookie);
     event.resetAfterDispatch();
 }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to