Closed Bug 1875424 Opened 2 years ago Closed 2 years ago

`mouseleave` should be fired on ancestors when the last `mouseover` target is removed and the cursor moved outside of the ancestors

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
124 Branch
Tracking Flags:
Tracking Status
firefox124 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(5 files)

Bug 1875424 - Make `EventStateManager` dispatch `mouseleave` events on ancestors even after the last `mouseover` target is removed r=smaug!,#dom-core
48 bytes, text/x-phabricator-request
Details | Review
Bug 1875424 - Make `OverOutElementsWrapper` private its members r=smaug!,#dom-core
48 bytes, text/x-phabricator-request
Details | Review
Bug 1875424 - Disable the new mouse/pointer boundary behavior in the late beta builds and the release builds r=smaug!,#dom-core
48 bytes, text/x-phabricator-request
Details | Review
Bug 1875424 - Make `PresShell::HandleEvent` flush pending synthetic mouse move at handling `eMouseMove` r=smaug!,#dom-core
48 bytes, text/x-phabricator-request
Details | Review
Bug 1875424 - Make `EventStateManager::ContentRemoved` enqueue a synthesized mouse move when `mouseout` target is removed r=smaug!,#dom-core
48 bytes, text/x-phabricator-request
Details | Review
No description provided.

Currently, EventStateManager clears the last mouseover element when it's
removed from the tree. Therefore, it does not fire mouselave events when
the cursor moves outside of ancestors which a mouseenter event is fired on.

This patch makes EventStateManager keep storing the parent of removing content
node which is an inclusive ancestor of the last mouseover target.

The following patch will make the code easier to read.

Depends on D193870

The members are public and the code accessing them are written with comments
since it's hard to read. So I think that they should be private members and
getters and setters should explain what they do with their names.

Depends on D199198

It seems that Chrome disables the new behavior in the release channel. For
backward compatibility, let's follow them for now.

https://bugs.chromium.org/p/chromium/issues/detail?id=1147998#c59

Depends on D199199

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/997aa1f6b4cc Make `EventStateManager` dispatch `mouseleave` events on ancestors even after the last `mouseover` target is removed r=smaug https://hg.mozilla.org/integration/autoland/rev/4e7412409dc3 Make `OverOutElementsWrapper` private its members r=smaug https://hg.mozilla.org/integration/autoland/rev/125cdc023eb9 Disable the new mouse/pointer boundary behavior in the late beta builds and the release builds r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44198 for changes under testing/web-platform/tests
Regressions: 1876531

Backed out for causing wpt failures on mouse_boundary_events_after_removing_last_over_element.html

Backout link

Push with failures

Failure log

Flags: needinfo?(masayuki)

Odd... Why does it fail only on Linux? And the failure means that the cursor once moved outside of the container but in the <body>...

Well, could be a timing issue of flushing the pending layout caused by the removal?

Flags: needinfo?(masayuki)

Ah, I guess that the fix of bug 1864654 is not enough. The test assumes that the mouseover should've already dispatched when PresShell handles the following eMouseMove caused by pointerMove(). However, currently PresShell::HandleEvent flushes the pending synthetic mouse move before eMouseDown and eMouseUp. So, probably it needs to flush the pending event at eMouseMove too.

Upstream PR merged by moz-wptsync-bot

Hmm, it could be a cause of the intermittent failure, but there is another reason at least because it still fails with the fix.
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=eQubxv3eSCi6ldsC_MlIBg.0&revision=12dbf96c24ef1d25bf47e40ef7669e015b35e27a

When it fails, the following events are fired before the click:

  • mouseover@div#child
  • mouseenter@body
  • mouseenter@div#grandparent
  • mouseenter@div#parent
  • mouseenter@div#child
  • mousemove@div#child

Those are expected, but mouseover on the new element, div#parent before the first leave after the click...

Okay, I got it. The synthetic mouse move may not be enqueued as expected. I'll make EventStateManager enqueue it when the mouseover target is removed.

mouse_boundary_events_after_removing_last_over_element.html expects that
a mouseover should be fired between click and mousemove, but it's
dispatched by a synthetic mouse move event caused by removing the click target.
The fix of bug 1864654 tried to fix this kind of issues, but it does not flush
pending event at eMouseMove. Therefore, the synthetic mouse move may not be
handled before handling eMouseMove which is caused by pointerMove() in the
test.

I guess that we need to make it flush before all events which are directly
caused by a user input. However, I don't want to do it for now because of
the risk.

Depends on D199607

I'm not exactly which PresShell::SynthesizeMouseMove() caller runs in most
cases, but according to the intermittent failure of
mouse_boundary_events_after_removing_last_over_element.html, it may not be
called in the case.

I guess the caller is PresShell::DidReflow()[1] which is called by a call of
PresShell::DoFlushLayout() in PresShell::DoFlushPendingNotifications()[2].
It runs only when the flush type is layout or larger. I guess that in the
failure cases, only FlushType::Frames or lower is called by somebody.

This patch makes EventStateManager::ContentRemoved ensures the synthetic
mouse move by itself. I think that this is reasonable and makes it never
depend on any changes of other places.

  1. https://searchfox.org/mozilla-central/rev/cee2c396081d950f9e3401113fb179999e404ab8/layout/base/PresShell.cpp#9564
  2. https://searchfox.org/mozilla-central/rev/cee2c396081d950f9e3401113fb179999e404ab8/layout/base/PresShell.cpp#4349

Depends on D199694

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/a85c761473a6 Make `EventStateManager` dispatch `mouseleave` events on ancestors even after the last `mouseover` target is removed r=smaug https://hg.mozilla.org/integration/autoland/rev/6106915f1a89 Make `OverOutElementsWrapper` private its members r=smaug https://hg.mozilla.org/integration/autoland/rev/16b759499f8a Disable the new mouse/pointer boundary behavior in the late beta builds and the release builds r=smaug https://hg.mozilla.org/integration/autoland/rev/6851b7bd79f0 Make `PresShell::HandleEvent` flush pending synthetic mouse move at handling `eMouseMove` r=smaug https://hg.mozilla.org/integration/autoland/rev/2353c9757dd8 Make `EventStateManager::ContentRemoved` enqueue a synthesized mouse move when `mouseout` target is removed r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44230 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
Regressions: 1877447
Regressions: 1943411
Component: DOM: Events → DOM: UI Events & Focus Handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: