search

web-platform-tests / interop

web-platform-tests Interop project
https://wpt.fyi/interop
318 stars 27 forks source link

The expectation of `pointerevent_after_target_removed.html` does not match the spec #380

Closed masayuki-nakano closed 7 months ago

masayuki-nakano commented 1 year ago

Test List

Rationale

I'm trying to fix the pointer event dispatching issue of pointerevent_after_target_removed.html?mouse in Firefox.

The test expects that pointerout and pointerleave are fired when_an element removed from the DOM tree when the pointer is under the element. One of the failure reason Firefox is, Firefox does not synthesize Pointer Events at changing element under the pointer.

Once I fix it, pointerevent_pointerout_no_pointer_movement.html starts failing in Firefox. This test checks that pointer events won't be fired when the visibility of an element under the point is toggled.

So, the former expects synthesized pointer events but the latter expects no synthesized pointer events. I think that the former behavior is better for web apps which want to update something when element under the pointer is changed without pointer state changes, and corresponding mouse events are fired since old browser versions. Therefore, the latter behavior blocks web apps to migrate mouse event listeners to pointer event listeners, but I'm not so familiar with Pointer Events, therefore, I'm not sure which test should be changed.

The test expects that pointerout and pointerleave are fired on the click target parent when the pointer moves out from the parent after removing the target from the DOM tree.

The pointerout and pointerleave are not defined as they should not be fired on orphan nodes. Therefore, it may be reasonable that those events are fired on the removed node (Safari works as so). However, it seems odd to dispatch at least pointerout on the (ex-)parent.

masayuki-nakano commented 1 year ago

Oh, the latter came from Mozilla, written by @EdgarChen, and reviewed by @smaug---- in bug 1529904.

(The former came from Chrome.)

EdgarChen commented 1 year ago

Right, the pointerevent_pointerout_no_pointer_movement.html was added for https://github.com/w3c/pointerevents/issues/457 which is for the cases where the pointer doesn't move, not triggering a pointerout event is the expected behavior per spec and https://github.com/w3c/pointerevents/issues/457.

However, pointerevent_after_target_removed.html is different, it moves the pointer to another element after the original element has been removed.

masayuki-nakano commented 1 year ago

Right, the pointerevent_pointerout_no_pointer_movement.html was added for w3c/pointerevents#457 which is for the cases where the pointer doesn't move, not triggering a pointerout event is the expected behavior per spec and w3c/pointerevents#457.

Thank you, Edgar!

However, pointerevent_after_target_removed.html is different, it moves the pointer to another element after the original element has been removed.

Oh, good point. I'll check the things and close this issue if I misunderstand something.

masayuki-nakano commented 1 year ago

I updated the first comment. So, the tests are not incongruous. However, I still think that the expectation of pointerevent_after_target_removed.html is not reasonable.

The test listens to pointerenter, pointerover, pointerout and pointerleave on the parent of the clicking element. The spec does not say that stop dispatching the events on the orphan nodes (after removed from the DOM tree). Therefore, dispatching pointerout and pointerleave on the removed element is fine and Safari does it. However, browsers may have already lost the ex-ancestor information of the orphan nodes. Therefore, it's hard to dispatch pointerleave on ex-ancestors.

I think that the spec should define the behavior in this case first.

EdgarChen commented 1 year ago

The test expects to get pointerenter, pointerover, pointerout and pointerleave dispatched on the div#parent.

Test generates following pointer actions in https://github.com/web-platform-tests/wpt/blob/8f3e51e7eaf38d99a7b6316b010cd673eb25d0ff/pointerevents/pointerevent_after_target_removed.html#L68-L75

let actions = new test_driver.Actions()
    .addPointer("TestPointer", pointer_type)
    .pointerMove(0, 0, {origin: parent})

Pointer is moved into the hit test boundaries of div#child, per spec, pointerenter is dispatched on both div#child and div#parent. And pointerover is dispatched on div#child only.

    .pointerDown()
    .pointerUp()

Now div#child is removed (either in pointerdown or pointerup event handler), but the pointer isn't moved, so no pointer event should be dispatched per https://github.com/w3c/pointerevents/issues/457.

    .pointerMove(0, 0, {origin: done})

Now pointer moves from div#parent to div#other, given that the pointer has moved out of the hit test boundaries of div#parent, should a pointerout or pointerleave event be dispatched to it?

In any case, it would be good if spec can define the behavior more clearly.

    .pointerDown()
    .pointerUp();

div#other receives click event, test end.

jgraham commented 1 year ago

@nt1m Do you have an option on this from the WebKit point of view? @flackr Do you have an opinion from the Chromium point of view?

nt1m commented 1 year ago

cc @aprotyas

flackr commented 1 year ago

I think the spec is pretty unclear about how to track whether a pointer is "entering" or "leaving" an element after a dom change. Each browser seems to have subtly different behaviors. I've put together a few cases in a demo:

  1. The green element is reparented to the body on click.
  2. The red element is reordered within the same parent on click.
  3. The purple element is removed on click.

In chrome, it seems that we keep tracking the node the pointer was over even if it was removed and readded and dispatch boundary events when the pointer moves for all changes from that node to the node the pointer is now over. In firefox, when the node is removed it seems we no longer consider the pointer over it. Reparenting doesn't dispatch any leave events but the next move re-enters all elements. Safari is similar to chrome, except still fires an event to the removed node.

All browsers have an experience where the enter and leave events can be mismatched as leaving relating to structural changes is invisible to the application. This is the issue being raised in crbug.com/1147998. One reasonable way we could fix this is when the node the pointer is over is removed, consider the still attached ancestor to be the node the pointer is now over.

As for whether or not we should fire events at removed nodes, while we do have precedent for this with touch events (demo) we don't for pointerevents.

So my proposal would be a behavior that no browser currently has for pointer events (as far as I can tell), but would ensure that the count of pointers would be correct. That being, when a node is detached, we track the nearest still attached ancestor of the previous node the pointer was over so that on the next move we don't re-enter any of the still attached ancestors if we're still over them.

mustaqahmed commented 1 year ago

In chrome, it seems that we keep tracking the node the pointer was over even if it was removed and readded and dispatch boundary events when the pointer moves for all changes from that node to the node the pointer is now over. In firefox, when the node is removed it seems we no longer consider the pointer over it. Reparenting doesn't dispatch any leave events but the next move re-enters all elements. Safari is similar to chrome, except still fires an event to the removed node.

Thanks for the concise summary of the differences. To add some details, Chrome remembers only the node under the pointer and handles all boundary event dispatches here (ignoring PE vs ME complications for sake of ease). Firefox diff seems to be just what you mentioned: forget a moved/removed node then re-hittest.

So my proposal would be a behavior that no browser currently has for pointer events (as far as I can tell), but would ensure that the count of pointers would be correct. That being, when a node is detached, we track the nearest still attached ancestor of the previous node the pointer was over so that on the next move we don't re-enter any of the still attached ancestors if we're still over them.

The big comment in the Chrome code above reminded me that in the past we found that "dynamically tracking the nearest attached ancestor" is not without its own issues: if we reorder (or even just reattach at the same point) an ancestor of the node under pointer, it will cause firing a sequence of redundant leave/enter events that we don't see in Chrome today. We concluded that the non-trivial cost of dynamic tracking in a deep DOM doesn't worth it: we would need to remember the whole ancestor chain and update it on every change in kIsConnectedFlag.

Let's discuss this in tomorrow's PEWG meeting.

flackr commented 1 year ago

Firefox diff seems to be just what you mentioned: forget a moved/removed node then re-hittest.

To be clear, every browser re-hittests on move, the difference is that in firefox there is no previous node so it re-enters every ancestor.

The big comment in the Chrome code above reminded me that in the past we found that "dynamically tracking the nearest attached ancestor" is not without its own issues: if we reorder (or even just reattach at the same point) an ancestor of the node under pointer, it will cause firing a sequence of redundant leave/enter events that we don't see in Chrome today.

Which is similar to, but fewer events than the current behavior on firefox.

We concluded that the non-trivial cost of dynamic tracking in a deep DOM doesn't worth it: we would need to remember the whole ancestor chain and update it on every change in kIsConnectedFlag.

I think it's actually much more trivial than this. mouse_press_node_ in blink already has this behavior that when the current mouse_pressnode is removed it automatically switches it to track the nearest still attached ancestor (see MouseEventManager::NodeWillBeRemoved). We could do the same without needing to remember the whole ancestor chain because any changes to that chain will fire NodeWillBeRemoved so it can use the node's ancestor chain.

smaug---- commented 1 year ago

when a node is detached, we track the nearest still attached ancestor of the previous node the pointer was over

I think that could work. Need to be careful of course with shadow DOM, ancestor here should mean the same thing as it does when constructing DOM Event path.

mustaqahmed commented 1 year ago

Let's discuss during today's PEWG meeting the following suggested changes to the test, now that we seem to have reached consensus on @flackr's proposal above:

  1. In the current test, the pointer enters a node (so also enters its parent) then deletes the node then exits the parent. So the current assumption is correct that at the parent element, both (a) 'enter' vs 'leave' events are paired and (b) 'over' and 'out' events are paired. Right?

  2. The test currently also assumes a relative order (at the parent) between 'enter/leave' and 'over/out' events. This matches corresponding UIEvents WPTs (mousemove-across.html and mousemove-between.html). But PointerEvent spec does not specify this (nor does UIEvents spec IIUC). Is it okay to remove this assumption from this particular test?

  3. We need to add an assertion on boundary events received at the deleted element. Currently this is missing.

smaug---- commented 1 year ago

One related thing to think about is whether changing the slot attribute of a slotted element so that it gets to a different place in the flattened tree has similar issues. (or changing the name attribute of a slot element)

mustaqahmed commented 1 year ago

As per PEWG discussion today:

flackr commented 1 year ago
  • @flackr suggested adding a test where the pointer remains stationary after deletion.

To clarify, I meant we should have a test where the pointer moves, but remains over the parent (rather than moving to another element). If the pointer never moves no subsequent events should be fired.

mustaqahmed commented 1 year ago

We have updated pointerevent_after_target_removed.html and added a new WPT pointerevent_after_target_appended.html to cover the discussion points in this thread. I am closing this issue as "done". @masayuki-nakano, please let us know if we missed anything.

masayuki-nakano commented 1 year ago

FYI: I filed an issue to UI Events about mouseover, mouseout, etc in the same situations. https://github.com/w3c/uievents/issues/359

masayuki-nakano commented 1 year ago

We have updated pointerevent_after_target_removed.html and added a new WPT pointerevent_after_target_appended.html to cover the discussion points in this thread. I am closing this issue as "done". @masayuki-nakano, please let us know if we missed anything.

Thank you for your work and sorry for the long delay.

"pointer events from mouse received before/after child removal at pointerdown" case now gets: Expectation Chrome Firefox Safari
pointerover@child pointerover@child pointerover@child pointerover@child
pointerenter@parent pointerenter@parent pointerenter@parent pointerenter@parent
pointerenter@child pointerenter@child pointerenter@child pointerenter@child
pointerdown@child pointerdown@child pointerdown@child pointerdown@child
(child-removed) (child-removed) (child-removed) (child-removed)
pointerover@parent pointerover@parent
pointerenter@parent
pointerup@parent pointerup@parent pointerup@parent pointerup@parent
pointerout@child
pointerleave@child
pointerover@parent pointerover@parent
pointerenter@parent pointerenter@parent
pointerdown@parent pointerdown@parent pointerdown@parent pointerdown@parent
pointerup@parent pointerup@parent pointerup@parent pointerup@parent
pointerout@parent pointerout@parent pointerout@parent pointerout@parent
pointerleave@parent pointerleave@parent pointerleave@parent pointerleave@parent

So, the redundant pointerout and pointerleave on the removed child on Safari should be the Safari's issue.

And also the redundant pointerenter on the parent on all browsers should be fixed in the browsers-side because mouseenter has already been fired on the parent, so, it's fired without mouseleave is odd from the event purpose point of view.

On the other hand, the even order of pointerover and pointerup on the parent seems terrible thing. On Gecko, when the element which is under the pointer is removed, it's queued to synthesize internal mousemove to run at next vsync to synthesize mouseover, mouseout, mouseenter, mouseleave, pointerover, pointerout, pointerenter and poineterleave. (FYI the synthesized internal mousemove is not fired on the DOM tree actually.)

Mozilla could change the behavior to synthesize the internal mousemove immediately`, but it could make damage to the DOM tree mutations under the pointer. So, it might be nicer the spec to allow this hack. I guess Safari does same thing according to the result.

(Should I file new issue for this? Or should we keep using this issue to avoid to disperse the series of this discussion?)

masayuki-nakano commented 1 year ago

The other results:

pointer events from mouse received before/after child removal at pointerup:

Expectation Chrome Firefox Safari
pointerover@child pointerover@child pointerover@child pointerover@child
pointerenter@parent pointerenter@parent pointerenter@parent pointerenter@parent
pointerenter@child pointerenter@child pointerenter@child pointerenter@child
pointerdown@child pointerdown@child pointerdown@child pointerdown@child
pointerup@child pointerup@child pointerup@child pointerup@child
pointerout@child
pointerleave@child
(child-removed) (child-removed) (child-removed) (child-removed)
pointerover@parent pointerover@parent pointerover@parent pointerover@parent
pointerenter@parent pointerenter@parent pointerenter@parent
pointerdown@parent pointerdown@parent pointerdown@parent pointerdown@parent
pointerup@parent pointerup@parent pointerup@parent pointerup@parent
pointerout@parent pointerout@parent pointerout@parent pointerout@parent
pointerleave@parent pointerleave@parent pointerleave@parent pointerleave@parent

mouse events from mouse received before/after child removal at pointerdown:

Expectation Chrome Firefox Safari
mouseover@child mouseover@child mouseover@child mouseover@child
mouseenter@parent mouseenter@parent mouseenter@parent mouseenter@parent
mouseenter@child mouseenter@child mouseenter@child mouseenter@child
(child-removed) (child-removed) (child-removed) (child-removed)
mouseover@parent
mousedown@parent mousedown@parent mousedown@child
mouseover@parent mouseover@parent
mouseenter@parent mouseenter@parent
mouseup@parent mouseup@parent mouseup@parent mouseup@parent
mouseout@child
mouseleave@child
mouseover@parent
mouseenter@parent
mousedown@parent mousedown@parent mousedown@parent mousedown@parent
mouseup@parent mouseup@parent mouseup@parent mouseup@parent
mouseout@parent mouseout@parent mouseout@parent mouseout@parent
mouseleave@parent mouseleave@parent mouseleave@parent mouseleave@parent

mouse events from mouse received before/after child removal at pointerup:

Expectation Chrome Firefox Safari
mouseover@child mouseover@child mouseover@child mouseover@child
mouseenter@parent mouseenter@parent mouseenter@parent mouseenter@parent
mouseenter@child mouseenter@child mouseenter@child mouseenter@child
mousedown@child mousedown@child mousedown@child mousedown@child
(child-removed) (child-removed) (child-removed) (child-removed)
mouseover@parent
mouseup@parent mouseup@parent mouseup@child
mouseout@child
mouseleave@child
mouseover@parent mouseover@parent mouseover@parent
mouseenter@parent mouseenter@parent mouseenter@parent
mousedown@parent mousedown@parent mousedown@parent mousedown@parent
mouseup@parent mouseup@parent mouseup@parent mouseup@parent
mouseout@parent mouseout@parent mouseout@parent mouseout@parent
mouseleave@parent mouseleave@parent mouseleave@parent mouseleave@parent

Oh, it seems that Chrome also has same hack to synthesize mouseout and mouseleave within different event loop after a while. So, it seems that the previous comment's result is a random result.

masayuki-nakano commented 1 year ago

Well, when an element is clicked (of course, by the user) and a pointerdown or pointerup listener removes the element, then, the parent is (in most cases) appear under the pointer. Then, Firefox stops dispatching the fallback mousedown or mouseup event on the parent element and Safari dispatches it within the removed child. However, only Chrome dispatches it in the parent which is not clicked by the user. I think that Firefox's behavior is better for users to stop kicking unexpected event listener with tricky code.

mustaqahmed commented 1 year ago

Re firing the fallback mouse event to the parent node: the WPT matches current spec requirement mentioned in this section:

Unless otherwise noted, the target of any mapped mouse event SHOULD be the same target as the respective pointer event unless the target is no longer participating in its ownerDocument's tree. In this case, the mouse event should be fired at the original target's nearest ancestor node Dispatching an event to a removed node is

Please file a new PointerEvents spec issue if you think this is a problem.

flackr commented 1 year ago

Mozilla could change the behavior to synthesize the internal mousemove immediately`, but it could make damage to the DOM tree mutations under the pointer. So, it might be nicer the spec to allow this hack. I guess Safari does same thing according to the result.

The expectation is not to dispatch any events immediately but to resolve / dispatch the boundary events before the next move / down / up event. For example, if you move the child node instead of remove it, it seems to be the case that we dispatch mouseover to the parent before mouseup: https://jsbin.com/hulakep/edit?html,css,js,output

masayuki-nakano commented 1 year ago

Re firing the fallback mouse event to the parent node: the WPT matches current spec requirement mentioned in this section:

Unless otherwise noted, the target of any mapped mouse event SHOULD be the same target as the respective pointer event unless the target is no longer participating in its ownerDocument's tree. In this case, the mouse event should be fired at the original target's nearest ancestor node Dispatching an event to a removed node is

Please file a new PointerEvents spec issue if you think this is a problem.

Filed https://github.com/w3c/pointerevents/issues/492 at least for clarifying that in the spec.

masayuki-nakano commented 1 year ago

Mozilla could change the behavior to synthesize the internal mousemove immediately`, but it could make damage to the DOM tree mutations under the pointer. So, it might be nicer the spec to allow this hack. I guess Safari does same thing according to the result.

The expectation is not to dispatch any events immediately but to resolve / dispatch the boundary events before the next move / down / up event. For example, if you move the child node instead of remove it, it seems to be the case that we dispatch mouseover to the parent before mouseup: https://jsbin.com/hulakep/edit?html,css,js,output

Ah, yeah, I meant "immediately" is about "must be before next mouseup or pointerup" in the cases of removing at mousedown or pointerdown.

If browsers need to guarantee the event order, they need to "flush" pending out/over/enter/leave events before dispatching mouseup or pointerup. I'm not sure whether it's reasonable for browsers.

flackr commented 1 year ago

Mozilla could change the behavior to synthesize the internal mousemove immediately`, but it could make damage to the DOM tree mutations under the pointer. So, it might be nicer the spec to allow this hack. I guess Safari does same thing according to the result.

The expectation is not to dispatch any events immediately but to resolve / dispatch the boundary events before the next move / down / up event. For example, if you move the child node instead of remove it, it seems to be the case that we dispatch mouseover to the parent before mouseup: https://jsbin.com/hulakep/edit?html,css,js,output

Ah, yeah, I meant "immediately" is about "must be before next mouseup or pointerup" in the cases of removing at mousedown or pointerdown.

If browsers need to guarantee the event order, they need to "flush" pending out/over/enter/leave events before dispatching mouseup or pointerup. I'm not sure whether it's reasonable for browsers.

My aim was to make the events seen by element removal consistent with the events seen when you move an element from under the cursor (except for the boundary events that would have been sent to the removed element per the ui-events requirement). It seems like Chrome and Firefox already flushes the expected boundary events when the element is moved out from under the cursor and all browsers do send the up event to the new target under the cursor: https://jsbin.com/hulakep/edit?js,output although interestingly the order of the pointerout / pointerover events on firefox are inconsistent with the mouseout/mouseover events.

Chrome Firefox Safari
pointerdown on b pointerdown on b pointerdown on b
mousedown on b mousedown on b mousedown on b
(Element moved) (Element moved) (Element moved)
pointerout on b
pointerover on a
mouseout on b mouseout on b
mouseleave on b mouseleave on b
mouseover on a mouseover on a
pointerup on a pointerup on a pointerup on a
mouseup on a mouseup on a mouseup on a
pointerout on b
pointerover on a

If the browser has already determined a new hit test target (as seems to have for all browsers) it's not unreasonable to also dispatch of boundary events. It would seem odd to dispatch pointerevents targeting an element that from the developer perspective the pointer has not entered. If we decided that browsers shouldn't determine a new target unless the pointer moved then I'd agree with you that imposing a hit test would be unreasonable. The boundary event dispatch is called out explicitly by pointerevent dispatch.

masayuki-nakano commented 1 year ago

@flackr Sorry for the delay to reply, I didn't realize your reply immediately.

Mozilla could change the behavior to synthesize the internal mousemove immediately`, but it could make damage to the DOM tree mutations under the pointer. So, it might be nicer the spec to allow this hack. I guess Safari does same thing according to the result.

The expectation is not to dispatch any events immediately but to resolve / dispatch the boundary events before the next move / down / up event. For example, if you move the child node instead of remove it, it seems to be the case that we dispatch mouseover to the parent before mouseup: https://jsbin.com/hulakep/edit?html,css,js,output

Ah, yeah, I meant "immediately" is about "must be before next mouseup or pointerup" in the cases of removing at mousedown or pointerdown. If browsers need to guarantee the event order, they need to "flush" pending out/over/enter/leave events before dispatching mouseup or pointerup. I'm not sure whether it's reasonable for browsers.

My aim was to make the events seen by element removal consistent with the events seen when you move an element from under the cursor (except for the boundary events that would have been sent to the removed element per the ui-events requirement). It seems like Chrome and Firefox already flushes the expected boundary events when the element is moved out from under the cursor

I think that the browsers use enough short time to flush pending boundary events for real user's operation speed in the typical environment. Firefox uses vsync (IIRC). Therefore, even if it's hard to reproduce it by your hands in 60fps display, it might not be so in 24fps mode like playing a movie in the display.

and all browsers do send the up event to the new target under the cursor:

Yeah, that should be because the DOM mutation itself should occur immediately during pointerdown.

If the browser has already determined a new hit test target (as seems to have for all browsers) it's not unreasonable to also dispatch of boundary events. It would seem odd to dispatch pointerevents targeting an element that from the developer perspective the pointer has not entered.

In strictly speaking, yes, it's odd. However, the boundary elements anyway should be dispatched because web apps may listen only to only the boundary events, not pointerup event in the case.

So, forcibly flushing pending boundary events dispatching before pointerdown, mousedown, pointerup and mouseup (and more?) seems reasonable for Pointer Events, but I'm not sure about the performance. I guess, no if we use this condition.

If we decided that browsers shouldn't determine a new target unless the pointer moved then I'd agree with you that imposing a hit test would be unreasonable. The boundary event dispatch is called out explicitly by pointerevent dispatch.

That would cause unexpected result for users. So I completely disagree. My point is, the boundary events may be delayed for the performance reasons, but if it should be fired immediately before some specific events, it should be defined.

flackr commented 1 year ago

If we decided that browsers shouldn't determine a new target unless the pointer moved then I'd agree with you that imposing a hit test would be unreasonable. The boundary event dispatch is called out explicitly by pointerevent dispatch.

That would cause unexpected result for users. So I completely disagree

I think you may have misinterpreted my meaning. I was not proposing we change this, merely saying that the expensive part of boundary event dispatch is typically the hit test and we already impose this requirement for the next event dispatch anyways.

So, forcibly flushing pending boundary events dispatching before pointerdown, mousedown, pointerup and mouseup (and more?) seems reasonable for Pointer Events, but I'm not sure about the performance. I guess, no if we use this condition.

My point is, the boundary events may be delayed for the performance reasons, but if it should be fired immediately before some specific events, it should be defined.

It sounds like we are in agreement then that it is a reasonable expectation? We can note it explicitly in the spec.

Are you able to construct any case today where boundary events are dispatched after other events targeting the element? If not - then this should be perfomance neutral right?

masayuki-nakano commented 10 months ago

Hi, I'm back this issue, and I'll fix the mouseenter dispatching bug of Gecko soon. However, the Gecko still fails the first test even after that. The result is as following:

Expectation Firefox (with patch)
pointerover@child pointerover@child
pointerenter@parent pointerenter@parent
pointerenter@child pointerenter@child
pointerdown@child pointerdown@child
(child-removed) (child-removed)
pointerover@parent
pointerup@parent pointerup@parent
pointerover@parent
pointerdown@parent pointerdown@parent
pointerup@parent pointerup@parent
pointerout@parent pointerout@parent
pointerleave@parent pointerleave@parent

The test does:

      let actions = new test_driver.Actions()
          .addPointer("TestPointer", pointer_type)
          .pointerMove(-30, -30, {origin: parent})
          .pointerDown()
          .pointerUp()
          .pointerMove(30, 30, {origin: parent})
          .pointerDown()
          .pointerUp()
          .pointerMove(0, 0, {origin: done})
          .pointerDown()
          .pointerUp()

So, during pointerDown() and pointerUp(), there is no pointermove. However, the test expects pointerover on the parent before pointerup.

The definition of pointerover is:

The user agent MUST fire a pointer event named pointerover when a pointing device is moved into the hit test boundaries of an element. Note that setPointerCapture() or releasePointerCapture() might have changed the hit test target. Also note that while a pointer is captured it is considered to be always inside the boundaries of the capturing element for the purpose of firing boundary events. The user agent MUST also fire this event prior to firing a pointerdown event for devices that do not support hover (see pointerdown).

So I believe that the new Gecko's behavior is right.

mustaqahmed commented 10 months ago

So the assumption here is that the first pointerup here does not imply a move into the hittest element, right? While this could be a valid assumption for a touch pointer (as a non-hovering pointer) for which up means the pointer has become inactive, I find "a pointerup implies a prior pointermove" more logical for a mouse pointer (as a hoverable pointer).

Both are perhaps acceptable perspectives, right? Because these are just a difference in interpretation.

However, if we now consider chorded button interactions, the distinction between pointermove and pointerup goes away and the proposed behavior (with the mentioned patch I mean) becomes questionable. Let me explain my point with an example.

Suppose the sequence of actions and DOM changes are as follows:

  1. move over the child element,
  2. left-button-down,
  3. middle-button-down,
  4. remove the child element,
  5. middle-button-up, and
  6. left-button-up.

At step 4, a pointermove@parent is fired while the event.button/buttons states imply this is a button release. As per the spec interpretation above, we would expect a pointerover@parent before the "chorded" pointermove@parent in this case because step 4 is exposed as a "move". How logical is this vs the proposed non-chorded-buttons case that pointerover@parent would be missing before the pointerup@parent? I don't see how web developers could reconcile this difference in exposed event sequences for two almost-identical physical interactions!

masayuki-nakano commented 9 months ago

@mustaqahmed Thank you for quick response and interesting perspective!

So the assumption here is that the first pointerup here does not imply a move into the hittest element, right?

I don't say so because the event point refers a point on the parent element. I just considered the right assumption from the current definition of pointerover.

While this could be a valid assumption for a touch pointer (as a non-hovering pointer) for which up means the pointer has become inactive, I find "a pointerup implies a prior pointermove" more logical for a mouse pointer (as a hoverable pointer).

Oh, about touch, the spec needs to be updated for clarifying the thing if pointer boundary events should be fired only when the pointer is acturely moved.

pointerup definition:

For input devices that do not support hover, the user agent MUST also fire a pointer event named pointerout followed by a pointer event named pointerleave after dispatching the pointerup event.

pointerout definition:

After firing the pointerup event for a device that does not support hover (see pointerup).

If pointerover has not been fired on the element, pointerout shouldn't be fired.

Both are perhaps acceptable perspectives, right? Because these are just a difference in interpretation.

I'm not sure. I think that current assumption of the test seems conflict with https://github.com/w3c/pointerevents/issues/457 and changing the behavior could break web apps in the wild.

However, if we now consider chorded button interactions, the distinction between pointermove and pointerup goes away and the proposed behavior (with the mentioned patch I mean) becomes questionable.

Really good point!

At step 4, a pointermove@parent is fired while the event.button/buttons states imply this is a button release. As per the spec interpretation above, we would expect a pointerover@parent before the "chorded" pointermove@parent in this case because step 4 is exposed as a "move". How logical is this vs the proposed non-chorded-buttons case that pointerover@parent would be missing before the pointerup@parent?

The pointerover definition just say "when a pointing device is moved" and pointermove event is defined as:

The user agent MUST fire a pointer event named pointermove when a pointer changes any properties that don't fire pointerdown or pointerup events. This includes any changes to coordinates, pressure, tangential pressure, tilt, twist, contact geometry (i.e. width and height) or chorded buttons.

In other words, despite the name, this event may not indicate a pointer move. So it's really error-prone, but my suggestion matches with the spec and I think that pointerover should not be fired in your example case.

I don't see how web developers could reconcile this difference in exposed event sequences for two almost-identical physical interactions!

Well, the difference between pointer boundary events and mouse boundary events made me confused and I guess a lot of developers would be confused too. I like the mouse boundary events behavior better, but I guess it's too late to change the basic rule of the pointer boundary behavior. So I think the chaos needs to be accepted and clearly mentioned in the spec if we prefer backward compatibility.

(I'm open for either result, I just talk for conforming to current spec definition.)

smaug---- commented 9 months ago

The spec is very clear that pointermove shouldn't be fired when pointerup is fired. https://w3c.github.io/pointerevents/#the-pointermove-event And https://w3c.github.io/pointerevents/#chorded-button-interactions is different thing.

mustaqahmed commented 9 months ago

I now see @masayuki-nakano's interpretation of the definition of pointerover as correct but I am still questioning the definition itself. I mentioned the chorded buttons case as an analogy, the main point that I wanted to raise as questionable is firing a pointerup to parent element when the pointerover/out sequence implies "the pointer is not over any element of the current DOM"! It was over the child element which has been removed.

Note that pointerdown and pointerup events explicitly define pointerover/out orders to avoid this scenario for non-hoverable pointers, at least that's my interpretation. EDITED a typo here.

about touch, the spec needs to be updated for clarifying the thing if pointer boundary events should be fired only when the pointer is acturely moved.

The suggestion here is to change the pointerdown text I just linked, right? Did we notice that there would no enter/over/out/leave events all for a tap to click?

A remotely related observation for records: the pointerenter definition calls out hovering pointerdown as a special case, and the pointerover definition should do the same (again, see the pointerdown text link above).

masayuki-nakano commented 9 months ago

firing a pointerup to parent element when the pointerover/out sequence implies "the pointer is not over any element of the current DOM"! It was over the child element which has been removed.

In my understanding, pointerup should be fired on the parent. And the further click event should be fired on the parent (in strictly speaking, the common ancestor of pointerdown and pointerup targets).

Note that pointerdown and pointerup events explicitly define pointerover/out orders to avoid this scenario for non-hoverable pointers, at least that's my interpretation. EDITED a typo here.

Well, I cannot say anything about non-hoverable pointers because I've not dug into the implementation of Gecko. I'd do that in this year.

about touch, the spec needs to be updated for clarifying the thing if pointer boundary events should be fired only when the pointer is acturely moved.

The suggestion here is to change the pointerdown text I just linked, right? Did we notice that there would no enter/over/out/leave events all for a tap to click?

Ah, that's it. I just couldn't find them before the comment, sorry for making you confused.

smaug---- commented 9 months ago

FYI, we're planning to change the test to follow the spec. If we want spec changes, that is a separate thing, but current wpts should follow the current spec.

mustaqahmed commented 9 months ago

Sounds good. Please file a spec bug for any spec clarification/discussion needed.

(I just filed https://github.com/w3c/pointerevents/issues/497 for the minor/unrelated spec discrepancy I noted above.)

masayuki-nakano commented 9 months ago

Now, the new expectation was merged.

FYI: When I fix the bugs of Firefox/Gecko, I added 2 WPTs which test the mouse or pointer boundary events with a pointer move after the click target is removed.

Both Chrome and Safari fail. About pointer boundary events, both of them dispatch pointerout event without a pointer move immediately after a click which removed the target. About mouse boundary events, Chrome dispatches unnecessary mouseleave events on the ancestors and that causes further unnecessary mouseenter events. Safari behaves just not fixed the storing issue of enter/leave target which was discussed here.

masayuki-nakano commented 9 months ago

Oh, I realized that there are these paragraphs in 4.1.3 Firing events using the PointerEvent interface:

Fire the event to the determined target. The user agent SHOULD treat the target as if the pointing device has moved over it from the previous target for the purpose of ensuring event ordering [UIEVENTS]. If the needs over event flag is set, an over event is needed even if the target element is the same.

Save the determined target as the previous target for the given pointer, and reset the needs over event flag to false. If the previous target at any point will no longer be connected [DOM], update the previous target to the nearest still connected [DOM] parent following the event path corresponding to dispatching events to the previous target, and set the needs over event flag to true.

These paragraphs explain the Chrome's behavior is right one. However, it's different from the conclusion from https://github.com/w3c/pointerevents/issues/477.

For the DOM mutation performance under a pointer, not dispatching any events makes browsers faster. However, I'm not sure whether it's so important or not.

@smaug---- @mustaqahmed @flackr

masayuki-nakano commented 9 months ago

Well, currently, I think that the paragraphs should be dropped from the spec because it's odd to dispatch pointer boundary events only when the target is removed. With this definition, when an element is appended and its region becomes underneath of the pointer, pointer boundary events are not fired. So, I feel they are inconsistent behavior from developers point of view.

mustaqahmed commented 9 months ago

Oh, I realized that there are these paragraphs in 4.1.3 Firing events using the PointerEvent interface:

...

These paragraphs explain the Chrome's behavior is right one. However, it's different from the conclusion from w3c/pointerevents#477.

Thanks @masayuki-nakano for paying close attention to this. To properly fix any gap, please elaborate any difference you spotted. Sorry if it is obvious, I am a bit lost between various boundary event discussions!

For the DOM mutation performance under a pointer, not dispatching any events makes browsers faster. However, I'm not sure whether it's so important or not.

I completely agree that not dispatching pointer event during DOM mutation is a good idea. The spec seems to support this in my opinion because 4.1.3 is about what happens during dispatch. What do you think?

With this definition, when an element is appended and its region becomes underneath of the pointer, pointer boundary events are not fired.

My interpretation is that boundary events are correctly dispatched for appended elements because of the same reason above: the section is about what happens during dispatch. If DOM has been modified since previous event dispatch, "previous target" would be different from "current target" (the element appended underneath the pointer), forcing boundary event firing.

flackr commented 9 months ago

Well, currently, I think that the paragraphs should be dropped from the spec because it's odd to dispatch pointer boundary events only when the target is removed. With this definition, when an element is appended and its region becomes underneath of the pointer, pointer boundary events are not fired.

When an element is appended and becomes underneath the pointer for the next event dispatch, the previous target will differ from the target of the next dispatched event and we will dispatch boundary events to enter the new element.

masayuki-nakano commented 9 months ago

@mustaqahmed

Oh, I realized that there are these paragraphs in 4.1.3 Firing events using the PointerEvent interface: ... These paragraphs explain the Chrome's behavior is right one. However, it's different from the conclusion from w3c/pointerevents#477.

Thanks @masayuki-nakano for paying close attention to this. To properly fix any gap, please elaborate any difference you spotted. Sorry if it is obvious, I am a bit lost between various boundary event discussions!

Sorry, I'm also lost. I'm a new for Pointer Events, therefore, I'm not familiar with discussions before.

Initially, I was thinking that pointer boundary events should be fired same time as mouse boundary events for making migration from mouse events to pointer events easier for developers (and avoid misunderstanding the new API). However, each event type definition of pointer boundary event defines they should be fired only when a pointer move occurs and https://github.com/w3c/pointerevents/issues/457 and test for it considered as same as the event type definitions. Therefore, I understood as so. However, I found the paragraphs yesterday when I was investigating another issue. Therefore, I'm now confused.

For the DOM mutation performance under a pointer, not dispatching any events makes browsers faster. However, I'm not sure whether it's so important or not.

I completely agree that not dispatching pointer event during DOM mutation is a good idea. The spec seems to support this in my opinion because 4.1.3 is about what happens during dispatch. What do you think?

Ah, indeed, dispatching pointer boundary events at next input makes sense from the performance point of view. However, it means that if user stops moving pointer for a while, pointer boundary event listeners cannot change something visually immediately after the mutation but will do it at next input suddenly. It maybe looks odd.

With this definition, when an element is appended and its region becomes underneath of the pointer, pointer boundary events are not fired.

My interpretation is that boundary events are correctly dispatched for appended elements because of the same reason above: the section is about what happens during dispatch. If DOM has been modified since previous event dispatch, "previous target" would be different from "current target" (the element appended underneath the pointer), forcing boundary event firing.

I don't see any definition about "forcing boundary event firing" in current spec. Which part does make you think so?

@flackr

Well, currently, I think that the paragraphs should be dropped from the spec because it's odd to dispatch pointer boundary events only when the target is removed. With this definition, when an element is appended and its region becomes underneath of the pointer, pointer boundary events are not fired.

When an element is appended and becomes underneath the pointer for the next event dispatch, the previous target will differ from the target of the next dispatched event and we will dispatch boundary events to enter the new element.

Well, if dispatching pointer boundary events is better for Pointer Events users, I don't disagree with doing that. However, the problem is, I don't find the definition in current spec, and the each event type documentation is unclear about mutation cases. Therefore, at least they should be defined clearer.

flackr commented 9 months ago

Well, if dispatching pointer boundary events is better for Pointer Events users, I don't disagree with doing that. However, the problem is, I don't find the definition in current spec, and the each event type documentation is unclear about mutation cases. Therefore, at least they should be defined clearer.

I agree! This was exactly the problem with the ui-events spec as well. It doesn't say anything about the mutation case which resulted in different removal behavior between the browsers.

The problem that the text you are concerned about is fixing is a real one, described in https://issues.chromium.org/u/0/issues/40156858 . The TLDR is that when the element the cursor is over is removed, browsers did not dispatch pointerleave / pointerout to any of the ancestors because on the next event (which results in dispatching boundary events), those nodes no longer were in the ancestor chain of the removed node.

The updated text from https://github.com/w3c/pointerevents/pull/491 changes this so that the previous element that we dispatch boundary events from tracks the still connected ancestor, so that from the perspective of ancestor listeners the cursor movement makes sense (e.g. we don't send enter to a node the cursor had previously entered and not seen a leave sent to yet).

To go back to the specific example, let's say you have a single node:

<div id="a"></div>

The cursor moves over this node, so we dispatch boundary events and set it as the |previous target|. Then when a new node is moved under the cursor or added under the cursor the next event dispatch will dispatch boundary events from that |previous target| to the new current target. This behavior is not new - it is intended to be the same as the implied behavior from ui events per https://www.w3.org/TR/uievents/#events-mouseevent-event-order

I believe there may be browser differences as to whether these boundary events occur at the next movement or earlier, but the change in the spec text was only to make sure that when you have a removed node we still know which node the cursor was over per the previous dispatched boundary events.

mustaqahmed commented 7 months ago

Didn't we already agree above that the spec requires firing boundary events before down and up events too? For convenience, here is the relevant spec text (link):

Fire the event to the determined target. The user agent SHOULD treat the target as if the pointing device has moved over it from the previous target for the purpose of ensuring event ordering. If the needs over event flag is set, an over event is needed even if the target element is the same.

If so, the last two WPT changes merged above go against the spec in my opinion and should be reverted:

In case the source of the confusion is the following question (which I didn't directly answer, sorry):

I don't see any definition about "forcing boundary event firing" in current spec. Which part does make you think so?

Please see the paragraph right after the quoted spec text above: the needs over flag is there just for this.

@masayuki-nakano: Are we on the same page?

masayuki-nakano commented 7 months ago

Didn't we already agree above that the spec requires firing boundary events before down and up events too? For convenience, here is the relevant spec text (link):

Fire the event to the determined target. The user agent SHOULD treat the target as if the pointing device has moved over it from the previous target for the purpose of ensuring event ordering. If the needs over event flag is set, an over event is needed even if the target element is the same.

Ah, yeah, the text seems reasonable for new inserted element under the pointer.

If so, the last two WPT changes merged above go against the spec in my opinion and should be reverted:

* [this one](https://github.com/web-platform-tests/wpt/pull/45192) just landed, and

* [this one](https://github.com/web-platform-tests/wpt/pull/44199) landed a while ago.

Well, for the latter, yes, it should be reverted. I'm still not 100% sure for the former, but I guess it's so. (ccing @EdgarChen)

mustaqahmed commented 7 months ago

Changes to both the spec and the WPT landed yesterday.

masayuki-nakano commented 7 months ago

Changes to both the spec and the WPT landed yesterday.

Thank you very much, and sorry for I couldn't do that because I have some urgent bug fixes...

mustaqahmed commented 7 months ago

No worries. Let us know if Sec4.1.3 still needs any update.