Commit f73b047b authored by Joey Arhar's avatar Joey Arhar Committed by Chromium LUCI CQ

Fire capture before bubble handlers at target

Before this patch, event handlers registered at the target of the event,
which get fired during the AT_TARGET event phase, get fired in order of
when they are registered without any regard for which handlers are
capture and which ones are bubble.

This patch fires all capture handlers before bubble handlers during the
AT_TARGET event phase.

WebKit implemented the same behavior here:
https://trac.webkit.org/changeset/236002/webkit

I2S: https://groups.google.com/a/chromium.org/forum/?fromgroups#!topic/blink-dev/30jv38uD_84

Fixed: 1052152
Change-Id: I87752225dbcef7d07fccf9a6999711bdf5fbad20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581814
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838108}
parent b24e2234
...@@ -224,11 +224,9 @@ DispatchEventResult EventDispatcher::Dispatch() { ...@@ -224,11 +224,9 @@ DispatchEventResult EventDispatcher::Dispatch() {
if (DispatchEventPreProcess(activation_target, if (DispatchEventPreProcess(activation_target,
pre_dispatch_event_handler_result) == pre_dispatch_event_handler_result) ==
kContinueDispatching) { kContinueDispatching) {
if (DispatchEventAtCapturing() == kContinueDispatching) { if (DispatchEventAtCapturing() == kContinueDispatching)
if (DispatchEventAtTarget() == kContinueDispatching)
DispatchEventAtBubbling(); DispatchEventAtBubbling();
} }
}
DispatchEventPostProcess(activation_target, DispatchEventPostProcess(activation_target,
pre_dispatch_event_handler_result); pre_dispatch_event_handler_result);
if (eventTiming) if (eventTiming)
...@@ -255,7 +253,8 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventPreProcess( ...@@ -255,7 +253,8 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventPreProcess(
inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() { inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() {
// Trigger capturing event handlers, starting at the top and working our way // Trigger capturing event handlers, starting at the top and working our way
// down. // down. When we get to the last one, the target, change the event phase to
// AT_TARGET and fire only the capture listeners on it.
event_->SetEventPhase(Event::kCapturingPhase); event_->SetEventPhase(Event::kCapturingPhase);
if (event_->GetEventPath().GetWindowEventContext().HandleLocalEvents( if (event_->GetEventPath().GetWindowEventContext().HandleLocalEvents(
...@@ -263,8 +262,8 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() { ...@@ -263,8 +262,8 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() {
event_->PropagationStopped()) event_->PropagationStopped())
return kDoneDispatching; return kDoneDispatching;
for (wtf_size_t i = event_->GetEventPath().size() - 1; i > 0; --i) { for (wtf_size_t i = event_->GetEventPath().size(); i > 0; --i) {
const NodeEventContext& event_context = event_->GetEventPath()[i]; const NodeEventContext& event_context = event_->GetEventPath()[i - 1];
if (event_context.CurrentTargetSameAsTarget()) { if (event_context.CurrentTargetSameAsTarget()) {
event_->SetEventPhase(Event::kAtTarget); event_->SetEventPhase(Event::kAtTarget);
event_->SetFireOnlyCaptureListenersAtTarget(true); event_->SetFireOnlyCaptureListenersAtTarget(true);
...@@ -281,17 +280,12 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() { ...@@ -281,17 +280,12 @@ inline EventDispatchContinuation EventDispatcher::DispatchEventAtCapturing() {
return kContinueDispatching; return kContinueDispatching;
} }
inline EventDispatchContinuation EventDispatcher::DispatchEventAtTarget() {
event_->SetEventPhase(Event::kAtTarget);
event_->GetEventPath()[0].HandleLocalEvents(*event_);
return event_->PropagationStopped() ? kDoneDispatching : kContinueDispatching;
}
inline void EventDispatcher::DispatchEventAtBubbling() { inline void EventDispatcher::DispatchEventAtBubbling() {
// Trigger bubbling event handlers, starting at the bottom and working our way // Trigger bubbling event handlers, starting at the bottom and working our way
// up. // up. On the first one, the target, change the event phase to AT_TARGET and
// fire only the bubble listeners on it.
wtf_size_t size = event_->GetEventPath().size(); wtf_size_t size = event_->GetEventPath().size();
for (wtf_size_t i = 1; i < size; ++i) { for (wtf_size_t i = 0; i < size; ++i) {
const NodeEventContext& event_context = event_->GetEventPath()[i]; const NodeEventContext& event_context = event_->GetEventPath()[i];
if (event_context.CurrentTargetSameAsTarget()) { if (event_context.CurrentTargetSameAsTarget()) {
// TODO(hayato): Need to check cancelBubble() also here? // TODO(hayato): Need to check cancelBubble() also here?
......
...@@ -71,7 +71,6 @@ class EventDispatcher { ...@@ -71,7 +71,6 @@ class EventDispatcher {
Node* activation_target, Node* activation_target,
EventDispatchHandlingState*&); EventDispatchHandlingState*&);
EventDispatchContinuation DispatchEventAtCapturing(); EventDispatchContinuation DispatchEventAtCapturing();
EventDispatchContinuation DispatchEventAtTarget();
void DispatchEventAtBubbling(); void DispatchEventAtBubbling();
void DispatchEventPostProcess(Node* activation_target, void DispatchEventPostProcess(Node* activation_target,
EventDispatchHandlingState*); EventDispatchHandlingState*);
......
This is a testharness.js-based test.
FAIL Dispatch additional events inside an event listener assert_array_equals: actual_targets lengths differ, expected array [object "[object Window]", Document node with 2 children, Element node <html><head><meta charset="utf-8">
<title> Dispatch addit..., Element node <body><div id="log"></div>
<table id="table" border="1" ..., Element node <table id="table" border="1" style="display: none">
<..., Element node <tbody id="table-body">
<tr id="table-row">
<..., Element node <tr id="parent">
<td id="target">Over the river, ..., Element node <td id="target">Over the river, Charlie</td>, [...], [...], [...], [...], [...], [...], [...], [...], [...]] length 17, got [object "[object Window]", Document node with 2 children, Element node <html><head><meta charset="utf-8">
<title> Dispatch addit..., Element node <body><div id="log"></div>
<table id="table" border="1" ..., Element node <table id="table" border="1" style="display: none">
<..., Element node <tbody id="table-body">
<tr id="table-row">
<..., Element node <tr id="parent">
<td id="target">Over the river, ..., Element node <td id="target">Over the river, Charlie</td>, [...], [...], [...], [...], [...], [...], [...], [...]] length 16
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Event-dispatch-listener-order assert_array_equals: expected property 4 to be "capturing SPAN" but got "bubbling SPAN" (expected array ["capturing SECTION", "capturing DIV", "capturing #document-fragment", "capturing P", "capturing SPAN", "bubbling SPAN", "bubbling P", "bubbling #document-fragment", "bubbling DIV", "bubbling SECTION"] got ["capturing SECTION", "capturing DIV", "capturing #document-fragment", "capturing P", "bubbling SPAN", "capturing SPAN", "bubbling P", "bubbling #document-fragment", "bubbling DIV", "bubbling SECTION"])
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Listeners are invoked in correct order (AT_TARGET phase) assert_array_equals: bubbles: true expected property 0 to be "capturing" but got "bubbling" (expected array ["capturing", "bubbling"] got ["bubbling", "capturing"])
Harness: the test ran to completion.
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" title="Joey Arhar" href="mailto:jarhar@chromium.org">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<script>
test(t => {
const element = document.createElement('div');
element.addEventListener('click', () => {
event.stopPropagation();
}, { capture: true });
element.addEventListener('click',
t.unreached_func('stopPropagation in the capture handler should have canceled this bubble handler.'));
element.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
});
</script>
This is a testharness.js-based test.
PASS Calling dispatchEvent(null).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (BeforeUnloadEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (CompositionEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (CustomEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (DeviceMotionEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (DeviceOrientationEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (DragEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (Event).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (Events).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (FocusEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (HashChangeEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (HTMLEvents).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (KeyboardEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (MessageEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (MouseEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (MouseEvents).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (StorageEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (SVGEvents).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (TextEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (TouchEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (UIEvent).
PASS If the event's initialized flag is not set, an InvalidStateError must be thrown (UIEvents).
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
FAIL Capturing event listeners should be called before non-capturing ones assert_array_equals: expected property 1 to be 3 but got 2 (expected array [1, 3, 2] got [1, 2, 3])
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL Capturing event listeners should be invoked before bubbling event listeners on the target without shadow trees assert_object_equals: property "0" expected "capturing" got "bubbling"
FAIL Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a shadow tree assert_object_equals: property "0" expected "capturing" got "bubbling"
FAIL Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched inside a doubly nested shadow tree assert_object_equals: property "0" expected "capturing" got "bubbling"
FAIL Capturing event listeners should be invoked before bubbling event listeners when an event is dispatched via a slot assert_object_equals: property "0" expected "capturing" got "bubbling"
FAIL 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 assert_object_equals: property "0" expected "capturing" got "bubbling"
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment