1877815 - Unexpected focus behaviour after pointerup event target is removed from the DOM

Status: RESOLVED FIXED

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

Description

[Ksenia Berezina [:ksenia]]

Attachment: linkedin.html

We received reports about this behaviour on Instagram and LinkedIn where after closing overlay popups content on the page gets selected with a slight move of a mouse (bug1837615). I've attached a reduced test case.

To reproduce:

1. Open the test case and click on the "Open Popup" button
2. Wait for the "pop up" and overlay to appear and click anywhere on the overlay
3. Move the mouse a bit and observe the behaviour

Expected:
Text doesn't get selected

Actual:
Text is selected

This only happens with a pointerdown event and not reproducible with a click event on the overlay. Both sites call focus() on the button element in the content before the popup is removed.
Also I've noticed that not reproducible if I just hide the popup with display:none, it has to be removed from the DOM.

Comment 1

[Dennis Schubert [:denschub]]

This is technically only a visual issue, but it looks incredibly broken, so let's track this as a webcompat-priority P2 for now.

Comment 2

[Dennis Schubert [:denschub]]

Actually, this is happening more frequently on Instagram, and it makes Firefox look really broken and might make the user feel like they have no control over the browser. Let's try to get this addressed.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Jan, do you think you could take a look, given that this has something to do with selection.
..and if not, please ping me and I'll try to find someone else :)

Comment 4

[Jan Jaeschke [:jjaschke]]

Removing the ni? for now since I'm on PTO until Feb 19. If urgent, ni? me again and I'll have a look.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, then, I'll take a look. I guess that this is an issue around nsIFrame::HandlePress.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1877815 - Make `PresShell::EventHandler::DispatchPrecedingPointerEvent` update event target after dispatching a pointer event r=smaug!,#dom-core

The reason of the bug is, no mouseup event is fired after the pointerup
event listener removed the target. Therefore, nsIFrame::HandleRelease() does
not run and nobody cleans up the drag state of the nsFrameSelection.

This is caused by that PresShell::EventHandler::DispatchPrecedingPointerEvent()
updates only event target frame of the following mouse event target if the
pointer event target was removed from the tree, however, the frame may not be
ready. In this case, PresShell::GetCurrentContent() will clear both current
event target content and frame because its composed document (nullptr) never
matches with the document for the PresShell. Therefore, it needs to update
the target too.

This patch makes all developers won't create similar bugs, this encapsulate
EventTargetData::mContent and EventTargetData::mFrame to make their setters
clean up or automatically check the relation.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1420589

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88c50740c908
Make `PresShell::EventHandler::DispatchPrecedingPointerEvent` update event target after dispatching a pointer event r=smaug

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/44516 for changes under testing/web-platform/tests

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/88c50740c908

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox123 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 9378931 [details]
Bug 1877815 - Make PresShell::EventHandler::DispatchPrecedingPointerEvent update event target after dispatching a pointer event r=smaug!,#dom-core

Beta/Release Uplift Approval Request

• User impact if declined: Some major web apps keep broken.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The fix makes it guarantee that the event target content <-> frame relation correct when they are set. However, this patch touches the path which handle all user input sent by OS. Therefore, not a low risk change.
• String changes made/needed:
• Is Android affected?: Yes

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9378931 [details]
Bug 1877815 - Make PresShell::EventHandler::DispatchPrecedingPointerEvent update event target after dispatching a pointer event r=smaug!,#dom-core

We are no longer in beta, this regressed in 59 and is risky to uplift, this should ride the 124 train,. Thanks