Changeset 236103 in webkit


Ignore:
Timestamp:
Sep 18, 2018 12:47:35 AM (6 years ago)
Author:
rniwa@webkit.org
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):

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r236031 r236103  
     12018-09-12  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Update composedPath to match the latest spec
     4        https://bugs.webkit.org/show_bug.cgi?id=180378
     5        <rdar://problem/42843004>
     6
     7        Reviewed by Darin Adler.
     8
     9        Rebaselined the test now that all test cases pass.
     10
     11        * web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt:
     12
    1132018-09-15  Rob Buis  <rbuis@igalia.com>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation-expected.txt

    r235881 r236103  
    11
    2 FAIL Event.composedPath() should return the same result even if DOM is mutated (1/2) assert_array_equals: lengths differ, expected 3 got 2
    3 FAIL Event.composedPath() should return the same result even if DOM is mutated (2/2) assert_array_equals: lengths differ, expected 5 got 2
     2PASS Event.composedPath() should return the same result even if DOM is mutated (1/2)
     3PASS Event.composedPath() should return the same result even if DOM is mutated (2/2)
    44

  • trunk/Source/WebCore/ChangeLog

    r236101 r236103  
     12018-09-12  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Update composedPath to match the latest spec
     4        https://bugs.webkit.org/show_bug.cgi?id=180378
     5        <rdar://problem/42843004>
     6
     7        Reviewed by Darin Adler.
     8
     9        This patch makes the check for whether a given node in the event path be included in composedPath
     10        pre-determined at the time of the event dispatching per https://github.com/whatwg/dom/issues/525.
     11        This was a fix for the issue that if an event listener in a closed shadow tree removes a node in the
     12        same tree in the event path, then composedPath called on its shadow host, for example, will include
     13        the removed node since it's no longer in the closed shadow tree.
     14
     15        Naively, implementing this behavior would require remembering the original document or shadow root
     16        of each node in the event path as well as its parent shadow root, or worse which node is disclosed
     17        to every other node at the time of computing the event path.
     18
     19        This patch takes a more novel and efficient approach to implement the new behavior by adding a single
     20        integer indicating the number of closed-mode shadow root ancestors of each node in the event path.
     21        In computePathUnclosedToTarget, any node whose *depth* is greater than the context object is excluded.
     22
     23        Consider the following example:
     24        div ------- ShadowRoot (closed)
     25          +- span     +- slot
     26        If an event is dispatched on span, then the event path would be [span, slot, ShadowRoot, div]. Then
     27        the values of integers assigned to each node would be: [0, 1, 1, 0] respectively. When composedPath
     28        is called on span or div, slot and ShadowRoot are excluded because they have a greater *depth* value.
     29
     30        Unfortunately, this simplistic solution doesn't work when there are multiple shadow roots of the same
     31        depth through which an event is dispatched as in:
     32        section -- ShadowRoot (closed, SR2)
     33          |          +- slot (s2)
     34          +div ------ ShadowRoot (closed, SR1)
     35            +- span     +- slot (s1)
     36        If an event is dispatched on span, the event path would be [span, s1, SR1, div, s2, SR2, section].
     37        The values of integers assigned are: [0, 1, 1, 0, 1, 1, 0] respectively. When composedPath is called
     38        on SR1, the simplistic approach would include s2 and SR2, which would be wrong.
     39
     40        To account for this case, in computePathUnclosedToTarget, we traverse the event path upwards (i.e.
     41        ancestors) and downwards (i.e. descendants) from the context object and decrease the *allowed depth*
     42        of shadow trees when we traverse out of a shadow tree in either direction. When traversing upwards,
     43        therefore, moving out of a shadow root to its host would would decrease the allowed depth. When
     44        traversing dowards, moving from a slot element to its assigned node would decrease the allowed depth.
     45
     46        Note that the depths can be negative when a composed event is dispatched inside a closed shadow tree,
     47        and it gets out of its shadow host.
     48
     49        Unfortunately, the latest DOM specification has a bug and doesn't match the behavior of Chrome. This
     50        patch proposes a new algorithm which can be adopted in https://github.com/whatwg/dom/issues/684.
     51
     52        Test: imported/w3c/web-platform-tests/shadow-dom/event-composed-path-after-dom-mutation.html
     53
     54        * dom/EventContext.cpp:
     55        (WebCore::EventContext::EventContext):
     56        (WebCore::MouseOrFocusEventContext::MouseOrFocusEventContext):
     57        (WebCore::TouchEventContext::TouchEventContext):
     58        * dom/EventContext.h:
     59        (WebCore::EventContext::closedShadowDepth const): Added.
     60        * dom/EventPath.cpp:
     61        (WebCore::WindowEventContext::WindowEventContext):
     62        (WebCore::EventPath::buildPath): Compute the closed shadow tree's depths for each node in the path.
     63        (WebCore::computePathUnclosedToTarget const): Implemented the aforementioned algorithm.
     64        (WebCore::EventPath::EventPath):
     65
    1662018-09-17  Yusuke Suzuki  <utatane.tea@gmail.com>
    267

  • trunk/Source/WebCore/dom/EventContext.cpp

    r236002 r236103  
    3636namespace WebCore {
    3737
    38 EventContext::EventContext(Node* node, EventTarget* currentTarget, EventTarget* target)
    39     : m_node(node)
    40     , m_currentTarget(currentTarget)
    41     , m_target(target)
     38EventContext::EventContext(Node* node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth)
     39    : m_node { node }
     40    , m_currentTarget { currentTarget }
     41    , m_target { target }
     42    , m_closedShadowDepth { closedShadowDepth }
    4243{
    4344    ASSERT(!isUnreachableNode(m_target.get()));
    … …  
    6768}
    6869
    69 MouseOrFocusEventContext::MouseOrFocusEventContext(Node& node, EventTarget* currentTarget, EventTarget* target)
    70     : EventContext(&node, currentTarget, target)
     70MouseOrFocusEventContext::MouseOrFocusEventContext(Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth)
     71    : EventContext(&node, currentTarget, target, closedShadowDepth)
    7172{
    7273}
    … …  
    8889#if ENABLE(TOUCH_EVENTS)
    8990
    90 TouchEventContext::TouchEventContext(Node& node, EventTarget* currentTarget, EventTarget* target)
    91     : EventContext(&node, currentTarget, target)
     91TouchEventContext::TouchEventContext(Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth)
     92    : EventContext(&node, currentTarget, target, closedShadowDepth)
    9293    , m_touches(TouchList::create())
    9394    , m_targetTouches(TouchList::create())

  • trunk/Source/WebCore/dom/EventContext.h

    r236002 r236103  
    3939    using EventInvokePhase = EventTarget::EventInvokePhase;
    4040
    41     EventContext(Node*, EventTarget* currentTarget, EventTarget*);
     41    EventContext(Node*, EventTarget* currentTarget, EventTarget*, int closedShadowDepth);
    4242    virtual ~EventContext();
    4343
    … …  
    4545    EventTarget* currentTarget() const { return m_currentTarget.get(); }
    4646    EventTarget* target() const { return m_target.get(); }
     47    int closedShadowDepth() const { return m_closedShadowDepth; }
    4748
    4849    virtual void handleLocalEvents(Event&, EventInvokePhase) const;
    … …  
    5960    RefPtr<EventTarget> m_currentTarget;
    6061    RefPtr<EventTarget> m_target;
     62    int m_closedShadowDepth { 0 };
    6163};
    6264
    6365class MouseOrFocusEventContext final : public EventContext {
    6466public:
    65     MouseOrFocusEventContext(Node&, EventTarget* currentTarget, EventTarget*);
     67    MouseOrFocusEventContext(Node&, EventTarget* currentTarget, EventTarget*, int closedShadowDepth);
    6668    virtual ~MouseOrFocusEventContext();
    6769
    … …  
    8082class TouchEventContext final : public EventContext {
    8183public:
    82     TouchEventContext(Node&, EventTarget* currentTarget, EventTarget*);
     84    TouchEventContext(Node&, EventTarget* currentTarget, EventTarget*, int closedShadowDepth);
    8385    virtual ~TouchEventContext();
    8486

  • trunk/Source/WebCore/dom/EventPath.cpp

    r236002 r236103  
    3636class WindowEventContext final : public EventContext {
    3737public:
    38     WindowEventContext(Node&, DOMWindow&, EventTarget&);
     38    WindowEventContext(Node&, DOMWindow&, EventTarget&, int closedShadowDepth);
    3939private:
    4040    void handleLocalEvents(Event&, EventInvokePhase) const final;
    4141};
    4242
    43 inline WindowEventContext::WindowEventContext(Node& node, DOMWindow& currentTarget, EventTarget& target)
    44     : EventContext(&node, &currentTarget, &target)
     43inline WindowEventContext::WindowEventContext(Node& node, DOMWindow& currentTarget, EventTarget& target, int closedShadowDepth)
     44    : EventContext(&node, &currentTarget, &target, closedShadowDepth)
    4545{
    4646}
    … …  
    112112void EventPath::buildPath(Node& originalTarget, Event& event)
    113113{
    114     using MakeEventContext = std::unique_ptr<EventContext> (*)(Node&, EventTarget*, EventTarget*);
    115     MakeEventContext makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target) {
    116         return std::make_unique<EventContext>(&node, currentTarget, target);
     114    using MakeEventContext = std::unique_ptr<EventContext> (*)(Node&, EventTarget*, EventTarget*, int closedShadowDepth);
     115    MakeEventContext makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth) {
     116        return std::make_unique<EventContext>(&node, currentTarget, target, closedShadowDepth);
    117117    };
    118118    if (is<MouseEvent>(event) || event.isFocusEvent()) {
    119         makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target) -> std::unique_ptr<EventContext> {
    120             return std::make_unique<MouseOrFocusEventContext>(node, currentTarget, target);
     119        makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth) -> std::unique_ptr<EventContext> {
     120            return std::make_unique<MouseOrFocusEventContext>(node, currentTarget, target, closedShadowDepth);
    121121        };
    122122    }
    123123#if ENABLE(TOUCH_EVENTS)
    124124    if (is<TouchEvent>(event)) {
    125         makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target) -> std::unique_ptr<EventContext> {
    126             return std::make_unique<TouchEventContext>(node, currentTarget, target);
     125        makeEventContext = [] (Node& node, EventTarget* currentTarget, EventTarget* target, int closedShadowDepth) -> std::unique_ptr<EventContext> {
     126            return std::make_unique<TouchEventContext>(node, currentTarget, target, closedShadowDepth);
    127127        };
    128128    }
    … …  
    131131    Node* node = nodeOrHostIfPseudoElement(&originalTarget);
    132132    Node* target = node ? eventTargetRespectingTargetRules(*node) : nullptr;
     133    int closedShadowDepth = 0;
     134    // Depths are used to decided which nodes are excluded in event.composedPath when the tree is mutated during event dispatching.
     135    // They could be negative for nodes outside the shadow tree of the target node.
    133136    while (node) {
    134137        while (node) {
    135             m_path.append(makeEventContext(*node, eventTargetRespectingTargetRules(*node), target));
     138            m_path.append(makeEventContext(*node, eventTargetRespectingTargetRules(*node), target, closedShadowDepth));
    136139
    137140            if (is<ShadowRoot>(*node))
    … …  
    145148                    if (target) {
    146149                        if (auto* window = downcast<Document>(*node).domWindow())
    147                             m_path.append(std::make_unique<WindowEventContext>(*node, *window, *target));
     150                            m_path.append(std::make_unique<WindowEventContext>(*node, *window, *target, closedShadowDepth));
    148151                    }
    149152                }
    … …  
    154157            if (UNLIKELY(shadowRootOfParent)) {
    155158                if (auto* assignedSlot = shadowRootOfParent->findAssignedSlot(*node)) {
     159                    if (shadowRootOfParent->mode() != ShadowRootMode::Open)
     160                        closedShadowDepth++;
    156161                    // node is assigned to a slot. Continue dispatching the event at this slot.
    157162                    parent = assignedSlot;
    … …  
    166171            return;
    167172        node = shadowRoot.host();
     173        if (shadowRoot.mode() != ShadowRootMode::Open)
     174            closedShadowDepth--;
    168175        if (exitingShadowTreeOfTarget)
    169176            target = eventTargetRespectingTargetRules(*node);
    … …  
    252259
    253260// https://dom.spec.whatwg.org/#dom-event-composedpath
     261// Any node whose depth computed in EventPath::buildPath is greater than the context object is excluded.
     262// Because we can exit out of a closed shadow tree and re-enter another closed shadow tree via a slot,
     263// we decrease the *allowed depth* whenever we moved to a "shallower" (closer-to-document) tree.
    254264Vector<EventTarget*> EventPath::computePathUnclosedToTarget(const EventTarget& target) const
    255265{
    256266    Vector<EventTarget*> path;
    257     path.reserveInitialCapacity(m_path.size());
    258     const Node* targetNode = nullptr;
    259     if (is<Node>(target))
    260         targetNode = &downcast<Node>(target);
    261     else if (is<DOMWindow>(target)) {
    262         targetNode = downcast<DOMWindow>(target).document();
    263         ASSERT(targetNode);
    264     }
    265     for (auto& context : m_path) {
    266         auto* currentTarget = context->currentTarget();
    267         if (!is<Node>(currentTarget) || !targetNode || !targetNode->isClosedShadowHidden(downcast<Node>(*currentTarget)))
    268             path.uncheckedAppend(currentTarget);
    269     }
     267    auto pathSize = m_path.size();
     268    RELEASE_ASSERT(pathSize);
     269    path.reserveInitialCapacity(pathSize);
     270
     271    auto currentTargetIndex = m_path.findMatching([&target] (auto& context) {
     272        return context->currentTarget() == &target;
     273    });
     274    RELEASE_ASSERT(currentTargetIndex != notFound);
     275    auto currentTargetDepth = m_path[currentTargetIndex]->closedShadowDepth();
     276
     277    auto appendTargetWithLesserDepth = [&path] (const EventContext& currentContext, int& currentDepthAllowed) {
     278        auto depth = currentContext.closedShadowDepth();
     279        bool contextIsInsideInnerShadowTree = depth > currentDepthAllowed;
     280        if (contextIsInsideInnerShadowTree)
     281            return;
     282        bool movedOutOfShadowTree = depth < currentDepthAllowed;
     283        if (movedOutOfShadowTree)
     284            currentDepthAllowed = depth;
     285        path.uncheckedAppend(currentContext.currentTarget());
     286    };
     287
     288    auto currentDepthAllowed = currentTargetDepth;
     289    auto i = currentTargetIndex;
     290    do {
     291        appendTargetWithLesserDepth(*m_path[i], currentDepthAllowed);
     292    } while (i--);
     293    path.reverse();
     294
     295    currentDepthAllowed = currentTargetDepth;
     296    for (auto i = currentTargetIndex + 1; i < pathSize; ++i)
     297        appendTargetWithLesserDepth(*m_path[i], currentDepthAllowed);
     298   
    270299    return path;
    271300}
    … …  
    273302EventPath::EventPath(const Vector<Element*>& targets)
    274303{
     304    // FIXME: This function seems wrong. Why are we not firing events in the closed shadow trees?
    275305    for (auto* target : targets) {
    276306        ASSERT(target);
    277307        Node* origin = *targets.begin();
    278308        if (!target->isClosedShadowHidden(*origin))
    279             m_path.append(std::make_unique<EventContext>(target, target, origin));
     309            m_path.append(std::make_unique<EventContext>(target, target, origin, 0));
    280310    }
    281311}
    … …  
    286316        ASSERT(target);
    287317        ASSERT(!is<Node>(target));
    288         m_path.append(std::make_unique<EventContext>(nullptr, target, *targets.begin()));
     318        m_path.append(std::make_unique<EventContext>(nullptr, target, *targets.begin(), 0));
    289319    }
    290320}
Note: See TracChangeset for help on using the changeset viewer.