- Revision
- 236103
- Author
- [email protected]
- Date
- 2018-09-18 00:47:35 -0700 (Tue, 18 Sep 2018)
Log Message
Update composedPath to match the latest spec
https://bugs.webkit.org/show_bug.cgi?id=180378
<rdar://problem/42843004>
Reviewed by Darin Adler.
LayoutTests/imported/w3c:
Rebaselined the test now that all test cases pass.
* web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt:
Source/WebCore:
This patch makes the check for whether a given node in the event path be included in composedPath
pre-determined at the time of the event dispatching per https://github.com/whatwg/dom/issues/525.
This was a fix for the issue that if an event listener in a closed shadow tree removes a node in the
same tree in the event path, then composedPath called on its shadow host, for example, will include
the removed node since it's no longer in the closed shadow tree.
Naively, implementing this behavior would require remembering the original document or shadow root
of each node in the event path as well as its parent shadow root, or worse which node is disclosed
to every other node at the time of computing the event path.
This patch takes a more novel and efficient approach to implement the new behavior by adding a single
integer indicating the number of closed-mode shadow root ancestors of each node in the event path.
In computePathUnclosedToTarget, any node whose *depth* is greater than the context object is excluded.
Consider the following example:
div ------- ShadowRoot (closed)
+- span +- slot
If an event is dispatched on span, then the event path would be [span, slot, ShadowRoot, div]. Then
the values of integers assigned to each node would be: [0, 1, 1, 0] respectively. When composedPath
is called on span or div, slot and ShadowRoot are excluded because they have a greater *depth* value.
Unfortunately, this simplistic solution doesn't work when there are multiple shadow roots of the same
depth through which an event is dispatched as in:
section -- ShadowRoot (closed, SR2)
| +- slot (s2)
+div ------ ShadowRoot (closed, SR1)
+- span +- slot (s1)
If an event is dispatched on span, the event path would be [span, s1, SR1, div, s2, SR2, section].
The values of integers assigned are: [0, 1, 1, 0, 1, 1, 0] respectively. When composedPath is called
on SR1, the simplistic approach would include s2 and SR2, which would be wrong.
To account for this case, in computePathUnclosedToTarget, we traverse the event path upwards (i.e.
ancestors) and downwards (i.e. descendants) from the context object and decrease the *allowed depth*
of shadow trees when we traverse out of a shadow tree in either direction. When traversing upwards,
therefore, moving out of a shadow root to its host would would decrease the allowed depth. When
traversing dowards, moving from a slot element to its assigned node would decrease the allowed depth.
Note that the depths can be negative when a composed event is dispatched inside a closed shadow tree,
and it gets out of its shadow host.
Unfortunately, the latest DOM specification has a bug and doesn't match the behavior of Chrome. This
patch proposes a new algorithm which can be adopted in https://github.com/whatwg/dom/issues/684.
Test: imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation.html
* dom/EventContext.cpp:
(WebCore::EventContext::EventContext):
(WebCore::MouseOrFocusEventContext::MouseOrFocusEventContext):
(WebCore::TouchEventContext::TouchEventContext):
* dom/EventContext.h:
(WebCore::EventContext::closedShadowDepth const): Added.
* dom/EventPath.cpp:
(WebCore::WindowEventContext::WindowEventContext):
(WebCore::EventPath::buildPath): Compute the closed shadow tree's depths for each node in the path.
(WebCore::computePathUnclosedToTarget const): Implemented the aforementioned algorithm.
(WebCore::EventPath::EventPath):
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (236102 => 236103)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2018-09-18 06:36:36 UTC (rev 236102)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2018-09-18 07:47:35 UTC (rev 236103)
@@ -1,3 +1,15 @@
+2018-09-12 Ryosuke Niwa <[email protected]>
+
+ Update composedPath to match the latest spec
+ https://bugs.webkit.org/show_bug.cgi?id=180378
+ <rdar://problem/42843004>
+
+ Reviewed by Darin Adler.
+
+ Rebaselined the test now that all test cases pass.
+
+ * web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt:
+
2018-09-15 Rob Buis <[email protected]>
XMLHttpRequest::createResponseBlob() should create a Blob with type for empty response
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt (236102 => 236103)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt 2018-09-18 06:36:36 UTC (rev 236102)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt 2018-09-18 07:47:35 UTC (rev 236103)
@@ -1,4 +1,4 @@
-FAIL Event.composedPath() should return the same result even if DOM is mutated (1/2) assert_array_equals: lengths differ, expected 3 got 2
-FAIL Event.composedPath() should return the same result even if DOM is mutated (2/2) assert_array_equals: lengths differ, expected 5 got 2
+PASS Event.composedPath() should return the same result even if DOM is mutated (1/2)
+PASS Event.composedPath() should return the same result even if DOM is mutated (2/2)
Modified: trunk/Source/WebCore/ChangeLog (236102 => 236103)
--- trunk/Source/WebCore/ChangeLog 2018-09-18 06:36:36 UTC (rev 236102)
+++ trunk/Source/WebCore/ChangeLog 2018-09-18 07:47:35 UTC (rev 236103)
@@ -1,3 +1,68 @@
+2018-09-12 Ryosuke Niwa <[email protected]>
+
+ Update composedPath to match the latest spec
+ https://bugs.webkit.org/show_bug.cgi?id=180378
+ <rdar://problem/42843004>
+
+ Reviewed by Darin Adler.
+
+ This patch makes the check for whether a given node in the event path be included in composedPath
+ pre-determined at the time of the event dispatching per https://github.com/whatwg/dom/issues/525.
+ This was a fix for the issue that if an event listener in a closed shadow tree removes a node in the
+ same tree in the event path, then composedPath called on its shadow host, for example, will include
+ the removed node since it's no longer in the closed shadow tree.
+
+ Naively, implementing this behavior would require remembering the original document or shadow root
+ of each node in the event path as well as its parent shadow root, or worse which node is disclosed
+ to every other node at the time of computing the event path.
+
+ This patch takes a more novel and efficient approach to implement the new behavior by adding a single
+ integer indicating the number of closed-mode shadow root ancestors of each node in the event path.
+ In computePathUnclosedToTarget, any node whose *depth* is greater than the context object is excluded.
+
+ Consider the following example:
+ div ------- ShadowRoot (closed)
+ +- span +- slot
+ If an event is dispatched on span, then the event path would be [span, slot, ShadowRoot, div]. Then
+ the values of integers assigned to each node would be: [0, 1, 1, 0] respectively. When composedPath
+ is called on span or div, slot and ShadowRoot are excluded because they have a greater *depth* value.
+
+ Unfortunately, this simplistic solution doesn't work when there are multiple shadow roots of the same
+ depth through which an event is dispatched as in:
+ section -- ShadowRoot (closed, SR2)
+ | +- slot (s2)
+ +div ------ ShadowRoot (closed, SR1)
+ +- span +- slot (s1)
+ If an event is dispatched on span, the event path would be [span, s1, SR1, div, s2, SR2, section].
+ The values of integers assigned are: [0, 1, 1, 0, 1, 1, 0] respectively. When composedPath is called
+ on SR1, the simplistic approach would include s2 and SR2, which would be wrong.
+
+ To account for this case, in computePathUnclosedToTarget, we traverse the event path upwards (i.e.
+ ancestors) and downwards (i.e. descendants) from the context object and decrease the *allowed depth*
+ of shadow trees when we traverse out of a shadow tree in either direction. When traversing upwards,
+ therefore, moving out of a shadow root to its host would would decrease the allowed depth. When
+ traversing dowards, moving from a slot element to its assigned node would decrease the allowed depth.
+
+ Note that the depths can be negative when a composed event is dispatched inside a closed shadow tree,
+ and it gets out of its shadow host.
+
+ Unfortunately, the latest DOM specification has a bug and doesn't match the behavior of Chrome. This
+ patch proposes a new algorithm which can be adopted in https://github.com/whatwg/dom/issues/684.
+
+ Test: imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation.html
+
+ * dom/EventContext.cpp:
+ (WebCore::EventContext::EventContext):
+ (WebCore::MouseOrFocusEventContext::MouseOrFocusEventContext):
+ (WebCore::TouchEventContext::TouchEventContext):
+ * dom/EventContext.h:
+ (WebCore::EventContext::closedShadowDepth const): Added.
+ * dom/EventPath.cpp:
+ (WebCore::WindowEventContext::WindowEventContext):
+ (WebCore::EventPath::buildPath): Compute the closed shadow tree's depths for each node in the path.
+ (WebCore::computePathUnclosedToTarget const): Implemented the aforementioned algorithm.
+ (WebCore::EventPath::EventPath):
+
2018-09-17 Yusuke Suzuki <[email protected]>
[WTF] Use Semaphore and BinarySemaphore instead of dispatch_semaphore_t
Modified: trunk/Source/WebCore/dom/EventContext.cpp (236102 => 236103)
--- trunk/Source/WebCore/dom/EventContext.cpp 2018-09-18 06:36:36 UTC (rev 236102)
+++ trunk/Source/WebCore/dom/EventContext.cpp 2018-09-18 07:47:35 UTC (rev 236103)
@@ -35,10 +35,11 @@
namespace WebCore {
-EventContext::EventContext(Node* node, EventTarget* currentTarget, EventTarget* target)
- : m_node(node)
- , m_currentTarget(currentTarget)
- , m_target(target)
+EventContext::EventContext(Node* node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth)
+ : m_node { node }
+ , m_currentTarget { currentTarget }
+ , m_target { target }
+ , m_closedShadowDepth { closedShadowDepth }
{
ASSERT(!isUnreachableNode(m_target.get()));
}
@@ -66,8 +67,8 @@
return false;
}
-MouseOrFocusEventContext::MouseOrFocusEventContext(Node& node, EventTarget* currentTarget, EventTarget* target)
- : EventContext(&node, currentTarget, target)
+MouseOrFocusEventContext::MouseOrFocusEventContext(Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth)
+ : EventContext(&node, currentTarget, target, closedShadowDepth)
{
}
@@ -87,8 +88,8 @@
#if ENABLE(TOUCH_EVENTS)
-TouchEventContext::TouchEventContext(Node& node, EventTarget* currentTarget, EventTarget* target)
- : EventContext(&node, currentTarget, target)
+TouchEventContext::TouchEventContext(Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth)
+ : EventContext(&node, currentTarget, target, closedShadowDepth)
, m_touches(TouchList::create())
, m_targetTouches(TouchList::create())
, m_changedTouches(TouchList::create())
Modified: trunk/Source/WebCore/dom/EventContext.h (236102 => 236103)
--- trunk/Source/WebCore/dom/EventContext.h 2018-09-18 06:36:36 UTC (rev 236102)
+++ trunk/Source/WebCore/dom/EventContext.h 2018-09-18 07:47:35 UTC (rev 236103)
@@ -38,12 +38,13 @@
public:
using EventInvokePhase = EventTarget::EventInvokePhase;
- EventContext(Node*, EventTarget* currentTarget, EventTarget*);
+ EventContext(Node*, EventTarget* currentTarget, EventTarget*, int closedShadowDepth);
virtual ~EventContext();
Node* node() const { return m_node.get(); }
EventTarget* currentTarget() const { return m_currentTarget.get(); }
EventTarget* target() const { return m_target.get(); }
+ int closedShadowDepth() const { return m_closedShadowDepth; }
virtual void handleLocalEvents(Event&, EventInvokePhase) const;
@@ -58,11 +59,12 @@
RefPtr<Node> m_node;
RefPtr<EventTarget> m_currentTarget;
RefPtr<EventTarget> m_target;
+ int m_closedShadowDepth { 0 };
};
class MouseOrFocusEventContext final : public EventContext {
public:
- MouseOrFocusEventContext(Node&, EventTarget* currentTarget, EventTarget*);
+ MouseOrFocusEventContext(Node&, EventTarget* currentTarget, EventTarget*, int closedShadowDepth);
virtual ~MouseOrFocusEventContext();
Node* relatedTarget() const { return m_relatedTarget.get(); }
@@ -79,7 +81,7 @@
class TouchEventContext final : public EventContext {
public:
- TouchEventContext(Node&, EventTarget* currentTarget, EventTarget*);
+ TouchEventContext(Node&, EventTarget* currentTarget, EventTarget*, int closedShadowDepth);
virtual ~TouchEventContext();
enum TouchListType { Touches, TargetTouches, ChangedTouches };
Modified: trunk/Source/WebCore/dom/EventPath.cpp (236102 => 236103)
--- trunk/Source/WebCore/dom/EventPath.cpp 2018-09-18 06:36:36 UTC (rev 236102)
+++ trunk/Source/WebCore/dom/EventPath.cpp 2018-09-18 07:47:35 UTC (rev 236103)
@@ -35,13 +35,13 @@
class WindowEventContext final : public EventContext {
public:
- WindowEventContext(Node&, DOMWindow&, EventTarget&);
+ WindowEventContext(Node&, DOMWindow&, EventTarget&, int closedShadowDepth);
private:
void handleLocalEvents(Event&, EventInvokePhase) const final;
};
-inline WindowEventContext::WindowEventContext(Node& node, DOMWindow& currentTarget, EventTarget& target)
- : EventContext(&node, ¤tTarget, &target)
+inline WindowEventContext::WindowEventContext(Node& node, DOMWindow& currentTarget, EventTarget& target, int closedShadowDepth)
+ : EventContext(&node, ¤tTarget, &target, closedShadowDepth)
{
}
@@ -111,19 +111,19 @@
void EventPath::buildPath(Node& originalTarget, Event& event)
{
- using MakeEventContext = std::unique_ptr<EventContext> (*)(Node&, EventTarget*, EventTarget*);
- MakeEventContext makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target) {
- return std::make_unique<EventContext>(&node, currentTarget, target);
+ using MakeEventContext = std::unique_ptr<EventContext> (*)(Node&, EventTarget*, EventTarget*, int closedShadowDepth);
+ MakeEventContext makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth) {
+ return std::make_unique<EventContext>(&node, currentTarget, target, closedShadowDepth);
};
if (is<MouseEvent>(event) || event.isFocusEvent()) {
- makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target) -> std::unique_ptr<EventContext> {
- return std::make_unique<MouseOrFocusEventContext>(node, currentTarget, target);
+ makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth) -> std::unique_ptr<EventContext> {
+ return std::make_unique<MouseOrFocusEventContext>(node, currentTarget, target, closedShadowDepth);
};
}
#if ENABLE(TOUCH_EVENTS)
if (is<TouchEvent>(event)) {
- makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target) -> std::unique_ptr<EventContext> {
- return std::make_unique<TouchEventContext>(node, currentTarget, target);
+ makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth) -> std::unique_ptr<EventContext> {
+ return std::make_unique<TouchEventContext>(node, currentTarget, target, closedShadowDepth);
};
}
#endif
@@ -130,9 +130,12 @@
Node* node = nodeOrHostIfPseudoElement(&originalTarget);
Node* target = node ? eventTargetRespectingTargetRules(*node) : nullptr;
+ int closedShadowDepth = 0;
+ // Depths are used to decided which nodes are excluded in event.composedPath when the tree is mutated during event dispatching.
+ // They could be negative for nodes outside the shadow tree of the target node.
while (node) {
while (node) {
- m_path.append(makeEventContext(*node, eventTargetRespectingTargetRules(*node), target));
+ m_path.append(makeEventContext(*node, eventTargetRespectingTargetRules(*node), target, closedShadowDepth));
if (is<ShadowRoot>(*node))
break;
@@ -144,7 +147,7 @@
ASSERT(target);
if (target) {
if (auto* window = downcast<Document>(*node).domWindow())
- m_path.append(std::make_unique<WindowEventContext>(*node, *window, *target));
+ m_path.append(std::make_unique<WindowEventContext>(*node, *window, *target, closedShadowDepth));
}
}
return;
@@ -153,6 +156,8 @@
auto* shadowRootOfParent = parent->shadowRoot();
if (UNLIKELY(shadowRootOfParent)) {
if (auto* assignedSlot = shadowRootOfParent->findAssignedSlot(*node)) {
+ if (shadowRootOfParent->mode() != ShadowRootMode::Open)
+ closedShadowDepth++;
// node is assigned to a slot. Continue dispatching the event at this slot.
parent = assignedSlot;
}
@@ -165,6 +170,8 @@
if (!shouldEventCrossShadowBoundary(event, shadowRoot, originalTarget))
return;
node = shadowRoot.host();
+ if (shadowRoot.mode() != ShadowRootMode::Open)
+ closedShadowDepth--;
if (exitingShadowTreeOfTarget)
target = eventTargetRespectingTargetRules(*node);
}
@@ -251,32 +258,55 @@
#endif
// https://dom.spec.whatwg.org/#dom-event-composedpath
+// Any node whose depth computed in EventPath::buildPath is greater than the context object is excluded.
+// Because we can exit out of a closed shadow tree and re-enter another closed shadow tree via a slot,
+// we decrease the *allowed depth* whenever we moved to a "shallower" (closer-to-document) tree.
Vector<EventTarget*> EventPath::computePathUnclosedToTarget(const EventTarget& target) const
{
Vector<EventTarget*> path;
- path.reserveInitialCapacity(m_path.size());
- const Node* targetNode = nullptr;
- if (is<Node>(target))
- targetNode = &downcast<Node>(target);
- else if (is<DOMWindow>(target)) {
- targetNode = downcast<DOMWindow>(target).document();
- ASSERT(targetNode);
- }
- for (auto& context : m_path) {
- auto* currentTarget = context->currentTarget();
- if (!is<Node>(currentTarget) || !targetNode || !targetNode->isClosedShadowHidden(downcast<Node>(*currentTarget)))
- path.uncheckedAppend(currentTarget);
- }
+ auto pathSize = m_path.size();
+ RELEASE_ASSERT(pathSize);
+ path.reserveInitialCapacity(pathSize);
+
+ auto currentTargetIndex = m_path.findMatching([&target] (auto& context) {
+ return context->currentTarget() == ⌖
+ });
+ RELEASE_ASSERT(currentTargetIndex != notFound);
+ auto currentTargetDepth = m_path[currentTargetIndex]->closedShadowDepth();
+
+ auto appendTargetWithLesserDepth = [&path] (const EventContext& currentContext, int& currentDepthAllowed) {
+ auto depth = currentContext.closedShadowDepth();
+ bool contextIsInsideInnerShadowTree = depth > currentDepthAllowed;
+ if (contextIsInsideInnerShadowTree)
+ return;
+ bool movedOutOfShadowTree = depth < currentDepthAllowed;
+ if (movedOutOfShadowTree)
+ currentDepthAllowed = depth;
+ path.uncheckedAppend(currentContext.currentTarget());
+ };
+
+ auto currentDepthAllowed = currentTargetDepth;
+ auto i = currentTargetIndex;
+ do {
+ appendTargetWithLesserDepth(*m_path[i], currentDepthAllowed);
+ } while (i--);
+ path.reverse();
+
+ currentDepthAllowed = currentTargetDepth;
+ for (auto i = currentTargetIndex + 1; i < pathSize; ++i)
+ appendTargetWithLesserDepth(*m_path[i], currentDepthAllowed);
+
return path;
}
EventPath::EventPath(const Vector<Element*>& targets)
{
+ // FIXME: This function seems wrong. Why are we not firing events in the closed shadow trees?
for (auto* target : targets) {
ASSERT(target);
Node* origin = *targets.begin();
if (!target->isClosedShadowHidden(*origin))
- m_path.append(std::make_unique<EventContext>(target, target, origin));
+ m_path.append(std::make_unique<EventContext>(target, target, origin, 0));
}
}
@@ -285,7 +315,7 @@
for (auto* target : targets) {
ASSERT(target);
ASSERT(!is<Node>(target));
- m_path.append(std::make_unique<EventContext>(nullptr, target, *targets.begin()));
+ m_path.append(std::make_unique<EventContext>(nullptr, target, *targets.begin(), 0));
}
}
