Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify pointerleave and pointerout events when first pointer move after removing an element under the pointer #477

Closed
masayuki-nakano opened this issue Jul 21, 2023 · 17 comments
Assignees

Comments

@masayuki-nakano
Copy link

I worked on fixing the failure in Firefox of pointerevent_after_target_removed.html. However, its expectation does not make sense. See web-platform-tests/interop#380 for the detail.

The main issue is, the spec does not define pointerleave and pointerout should be fired on which element(s) or should not be fired at the first move after an element under the pointer is removed.

Some definitions indicate that pointerleave, pointerout, pointerenter and pointerover should not be fired immediately (without a pointer move) when an element under the pointer is removed by a DOM API or hidden/moved by a style change. Then, web apps may want to listen to pointerleave and pointerout at next pointer move to clear something set at pointerenter or pointerover.

The spec does not disallow to dispatch pointerout on removed element (which is the last pointerover'ed element). Safari does that, and it seems reasonable.

The big problem here is, pointerleave. pointerenter must have been fired on the removed element and its ancestors. From web apps point of view, pointerleave should be fired on all elements which pointerenter was fired on and are not ancestors of the element currently under the pointer. However, it sounds like terrible. To do that, browsers need to store all elements which the browser dispatched pointerenter, but this means that nearly root ancestors will be stored for a long time, e.g., the <body> element. And the dispatching code becomes complicated because DOM API may remove only some ancestors with re-connecting an ancestor to the same point.

@EdgarChen, @smaug----

@mustaqahmed
Copy link
Member

Below are my comments/questions to aid the discussion:

I worked on fixing the failure in Firefox of pointerevent_after_target_removed.html. However, its expectation does not make sense. See web-platform-tests/interop#380 for the detail.

Hmm, I added this test to match the expectation in a user reported Chrome bug. I haven't looked into all the details you mentioned above, but I can see that pointerover/out events were not mentioned in the Chrome bug, so I need to check if I accidentally made the test an "overfit"!

The main issue is, the spec does not define pointerleave and pointerout should be fired on which element(s) or should not be fired at the first move after an element under the pointer is removed.

Yes, and one reason is that the text in PointerEvents depends on the hand-wavy definition in UI Event Spec.

Some definitions indicate that pointerleave, pointerout, pointerenter and pointerover should not be fired immediately (without a pointer move) when an element under the pointer is removed by a DOM API or hidden/moved by a style change. Then, web apps may want to listen to pointerleave and pointerout at next pointer move to clear something set at pointerenter or pointerover.

The spec does not disallow to dispatch pointerout on removed element (which is the last pointerover'ed element). Safari does that, and it seems reasonable.

After the element is removed from DOM, my quick read of DOM event dispatch algorithm suggests that the algorithm shouldn't do anything at least because event.eventPhase becomes undefined in that case. The algorithm is a bit complicated, so please correct me if I missed anything here (which is very likely).

One question is: does WebKit dispatch the pointerout event right before the element is removed (and before any further physical movement of the pointer)?

The big problem here is, pointerleave. pointerenter must have been fired on the removed element and its ancestors. From web apps point of view, pointerleave should be fired on all elements which pointerenter was fired on and are not ancestors of the element currently under the pointer. However, it sounds like terrible. To do that, browsers need to store all elements which the browser dispatched pointerenter, but this means that nearly root ancestors will be stored for a long time, e.g., the <body> element. And the dispatching code becomes complicated because DOM API may remove only some ancestors with re-connecting an ancestor to the same point.

@EdgarChen, @smaug----

Chrome does not store all elements, instead keeps track of the last known "element under pointer" then infers about exited/enter elements using the common ancestor of previous vs current "element under pointer" in the current DOM. The big comment in the common ancestor code implies it's a legacy Chrome behavior to "match Firefox and IE".

@masayuki-nakano
Copy link
Author

I worked on fixing the failure in Firefox of pointerevent_after_target_removed.html. However, its expectation does not make sense. See web-platform-tests/interop#380 for the detail.

Hmm, I added this test to match the expectation in a user reported Chrome bug. I haven't looked into all the details you mentioned above, but I can see that pointerover/out events were not mentioned in the Chrome bug, so I need to check if I accidentally made the test an "overfit"!

The test expects that same kind of events are dispatched with same event order between mouse events and pointer events.

Historically, mouseout and mouseleave are fired when the element under the mouse cursor is removed/moved. This is defined as:

A user agent MUST dispatch this event when a pointing device is moved off of the boundaries of an element or when the element is moved to be no longer underneath the primary pointing device.

On the other hand, pointerout and poinerleave does not have the similar definition and this issue clearly indicates that browsers should not dispatch the events until the pointer is moved.

I.e., they are fired in the different rules, but the test expects same result between mouse events and pointer events.

Some definitions indicate that pointerleave, pointerout, pointerenter and pointerover should not be fired immediately (without a pointer move) when an element under the pointer is removed by a DOM API or hidden/moved by a style change. Then, web apps may want to listen to pointerleave and pointerout at next pointer move to clear something set at pointerenter or pointerover.
The spec does not disallow to dispatch pointerout on removed element (which is the last pointerover'ed element). Safari does that, and it seems reasonable.

After the element is removed from DOM, my quick read of DOM event dispatch algorithm suggests that the algorithm shouldn't do anything at least because event.eventPhase becomes undefined in that case. The algorithm is a bit complicated, so please correct me if I missed anything here (which is very likely).

I think that it's fine to dispatch DOM events in a document fragment. An orphan node can be treated as the root element of a document fragment. Then, event phase can be set as same as when the node is in the owner document. (I.e., if browsers dispatch a DOM event on an orphan node, only the target phase will appear.)

One question is: does WebKit dispatch the pointerout event right before the element is removed (and before any further physical movement of the pointer)?

According to the test result (only pointerenter is fired on the (ex-)parent), Safari dispatches pointerout after the node is removed (and probably at moving the pointer).

Chrome does not store all elements, instead keeps track of the last known "element under pointer" then infers about exited/enter elements using the common ancestor of previous vs current "element under pointer" in the current DOM. The big comment in the common ancestor code implies it's a legacy Chrome behavior to "match Firefox and IE".

Firefox does similar thing. Firefox stores both elements under the cursor which are targeted by mouseover and pointerover and compute ancestors to dispatch *leave events at dispatch. Therefore, once the DOM tree is changed from *enter events, elements may be targeted only by *leave or *enter.

I think that Chrome dispatches pointerout and pointerover immediately if the element under the pointer is removed. Therefore, reset the storage and store new elements under the pointer within the new DOM tree.

@flackr
Copy link
Contributor

flackr commented Aug 19, 2023

I left my recommendation in web-platform-tests/interop#380 (comment)

TLDR, i think that on node removal we should track the nearest still attached node as the one the pointer is over. This will result in correct pointer counts when we subsequently dispatch boundary events.

@patrickhlauke
Copy link
Member

patrickhlauke commented Sep 14, 2023

Decided during PEWG meeting at TPAC: Leaving this issue open until we have at least one browser that includes the proposed fix/behaviour

@mustaqahmed mustaqahmed added the needs-wpt Investigation whether the issue needs a wpt test has been done and wpt is missing label Oct 11, 2023
@mustaqahmed
Copy link
Member

We still need WPTs around shadow-DOM boundary and slot elements.

@smaug----
Copy link
Contributor

smaug---- commented Oct 11, 2023

For Shadow DOM if the removed node is topmost element in Shadow DOM, we probably want to find the nearest shadow-including inclusive ancestor Element, and otherwise just find parent the same way how event path would find a "parent".'

Or should we rely on event path?

@flackr
Copy link
Contributor

flackr commented Oct 25, 2023

It looks like step dispatching events defines how the event path is constructed. In the case of node removal we don't have a current event so we should follow the algorithm there for finding the nearest still attached parent that would be in the event path.

flackr added a commit to flackr/pointerevents that referenced this issue Oct 31, 2023
This explains how the previous target is tracked when the previous
target is removed from the DOM for w3c#477.
@flackr
Copy link
Contributor

flackr commented Nov 9, 2023

While reviewing the fix @mustaqahmed is working on, I also discovered that when we remove nodes during the dispatch of boundary events we still send events to the removed nodes.

E.g. load https://jsbin.com/rawogov/edit?html,css,js,output and move the mouse from the blue box up. The green removed box still gets an event.

I think this is also in violation of the ui-events section which states:

If the event target (e.g. the target element) is removed from the DOM during the mouse events sequence, the remaining events of the sequence MUST NOT be fired on that element.

@mustaqahmed
Copy link
Member

Thanks for pointing out the violation of the spec. Chrome had an old comment to investigate its boundary event dispatch algorithm, see the bug we just filed.

patrickhlauke pushed a commit that referenced this issue Nov 22, 2023
…#491)

* Explain how a removed DOM node should be handled for boundary events.

This explains how the previous target is tracked when the previous
target is removed from the DOM for #477.

* Use a more specific needs over event flag.
@patrickhlauke
Copy link
Member

Closing, just needs WPT

@patrickhlauke
Copy link
Member

From @mustaqahmed

Chrome supports this behavior now as an experimental web platform feature, so we now have one impl technically.

@mustaqahmed
Copy link
Member

Looks like @masayuki-nakano addded this WPT to cover the shadow-dom case: https://github.com/web-platform-tests/wpt/blob/master/pointerevents/pointerevent_pointer_boundary_events_after_removing_last_over_element.html

We still need a WPT for slot elements.

@smaug----
Copy link
Contributor

@masayuki-nakano would it be easy to just tweak those tests a tiny bit to cover also slots?

@masayuki-nakano
Copy link
Author

@masayuki-nakano would it be easy to just tweak those tests a tiny bit to cover also slots?

Yeah, I think so, but could you clarify what cases do you want?

@smaug----
Copy link
Contributor

I guess there should be tests for removing an assigned node (so removing a light DOM node, which is assigned to a slot), and also test a case where slot has assigned nodes and then the slot itself is removed from shadow DOM.

@masayuki-nakano
Copy link
Author

@mustaqahmed mustaqahmed self-assigned this May 24, 2024
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 11, 2024
This CL adds tests for two untested aspects of PointerEvents in slots:
- PE event sequence around slots when the shadow DOM remains unchanged.
- Boundary events after modifications to slotted elements.

This covers the last remaining WPT for:
w3c/pointerevents#477

Bug: 40156858

Change-Id: I9823595a0b15672d0395d4e117ccc6462c3e42f2
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 11, 2024
This CL adds tests for two untested aspects of PointerEvents in slots:
- PE event sequence around slots when the shadow DOM remains unchanged.
- Boundary events after modifications to slotted elements.

This covers the last remaining WPT for:
w3c/pointerevents#477

Bug: 40156858

Change-Id: I9823595a0b15672d0395d4e117ccc6462c3e42f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5598510
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313570}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 11, 2024
This CL adds tests for two untested aspects of PointerEvents in slots:
- PE event sequence around slots when the shadow DOM remains unchanged.
- Boundary events after modifications to slotted elements.

This covers the last remaining WPT for:
w3c/pointerevents#477

Bug: 40156858

Change-Id: I9823595a0b15672d0395d4e117ccc6462c3e42f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5598510
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313570}
@mustaqahmed
Copy link
Member

Done landing WPTs for slotted elements: web-platform-tests/wpt#46694

@mustaqahmed mustaqahmed removed the needs-wpt Investigation whether the issue needs a wpt test has been done and wpt is missing label Jun 11, 2024
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 11, 2024
Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 12, 2024
Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313946}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 12, 2024
Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313946}
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jun 14, 2024
…ted elements., a=testonly

Automatic update from web-platform-tests
WPT coverage for Pointer Events for slotted elements.

This CL adds tests for two untested aspects of PointerEvents in slots:
- PE event sequence around slots when the shadow DOM remains unchanged.
- Boundary events after modifications to slotted elements.

This covers the last remaining WPT for:
w3c/pointerevents#477

Bug: 40156858

Change-Id: I9823595a0b15672d0395d4e117ccc6462c3e42f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5598510
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313570}

--

wpt-commits: b8aab6837b61aacee4cbad1fbb6aeb3952af7b60
wpt-pr: 46694
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 14, 2024
…ted elements., a=testonly

Automatic update from web-platform-tests
WPT coverage for Pointer Events for slotted elements.

This CL adds tests for two untested aspects of PointerEvents in slots:
- PE event sequence around slots when the shadow DOM remains unchanged.
- Boundary events after modifications to slotted elements.

This covers the last remaining WPT for:
w3c/pointerevents#477

Bug: 40156858

Change-Id: I9823595a0b15672d0395d4e117ccc6462c3e42f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5598510
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313570}

--

wpt-commits: b8aab6837b61aacee4cbad1fbb6aeb3952af7b60
wpt-pr: 46694
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jun 16, 2024
…ted elements., a=testonly

Automatic update from web-platform-tests
WPT coverage for Pointer Events for slotted elements.

This CL adds tests for two untested aspects of PointerEvents in slots:
- PE event sequence around slots when the shadow DOM remains unchanged.
- Boundary events after modifications to slotted elements.

This covers the last remaining WPT for:
w3c/pointerevents#477

Bug: 40156858

Change-Id: I9823595a0b15672d0395d4e117ccc6462c3e42f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5598510
Commit-Queue: Mustaq Ahmed <mustaqchromium.org>
Reviewed-by: Robert Flack <flackrchromium.org>
Cr-Commit-Position: refs/heads/main{#1313570}

--

wpt-commits: b8aab6837b61aacee4cbad1fbb6aeb3952af7b60
wpt-pr: 46694

UltraBlame original commit: 60779ff43603360d82107deb6ca1cd519fa169d8
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 18, 2024
…to shadow-host and slot, a=testonly

Automatic update from web-platform-tests
PointerEvent WPT: add checks for events to shadow-host and slot

Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313946}

--

wpt-commits: a3831b7add5dc42c1daf38a93e827710f8839576
wpt-pr: 46697
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jun 19, 2024
…to shadow-host and slot, a=testonly

Automatic update from web-platform-tests
PointerEvent WPT: add checks for events to shadow-host and slot

Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaqchromium.org>
Reviewed-by: Robert Flack <flackrchromium.org>
Cr-Commit-Position: refs/heads/main{#1313946}

--

wpt-commits: a3831b7add5dc42c1daf38a93e827710f8839576
wpt-pr: 46697

UltraBlame original commit: 828793bbbe3b5a092eddaa5191a34d2c818deb35
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jun 19, 2024
…to shadow-host and slot, a=testonly

Automatic update from web-platform-tests
PointerEvent WPT: add checks for events to shadow-host and slot

Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaqchromium.org>
Reviewed-by: Robert Flack <flackrchromium.org>
Cr-Commit-Position: refs/heads/main{#1313946}

--

wpt-commits: a3831b7add5dc42c1daf38a93e827710f8839576
wpt-pr: 46697

UltraBlame original commit: 828793bbbe3b5a092eddaa5191a34d2c818deb35
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jun 21, 2024
…to shadow-host and slot, a=testonly

Automatic update from web-platform-tests
PointerEvent WPT: add checks for events to shadow-host and slot

Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313946}

--

wpt-commits: a3831b7add5dc42c1daf38a93e827710f8839576
wpt-pr: 46697
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 24, 2024
…to shadow-host and slot, a=testonly

Automatic update from web-platform-tests
PointerEvent WPT: add checks for events to shadow-host and slot

Related to:
w3c/pointerevents#477

Bug: 40156858
Change-Id: Ia7b952a48b89a6d932a3a4d71f36e8086d261c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5622053
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313946}

--

wpt-commits: a3831b7add5dc42c1daf38a93e827710f8839576
wpt-pr: 46697
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants