1635784 - Mouse cursor not update properly in Fission for some cases

Status: RESOLVED FIXED

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

Description

[Edgar Chen [:edgar]]

Like, https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/dom/events/EventStateManager.cpp#3882, it just checks remote target from parent process point of view, but we should also take into account if it is a fission remote target in content process.

And then we might also need to ensure that there is at most one BrowserParent whose TabSetsCursor is true at any time, https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/dom/ipc/BrowserParent.h#966-968.

Comment 1

[Chris Peterson [:cpeterson]]

Henri, will this bug be fixed by your focus targeting changes (bug 1617788)?

Comment 2

[Henri Sivonen (:hsivonen)]

(In reply to Chris Peterson [:cpeterson] from comment #1)

Henri, will this bug be fixed by your focus targeting changes (bug 1617788)?

Most likely this bug won't be affected by that one.

Comment 3

[Edgar Chen [:edgar]]

Steps to reproduce

1. Load https://codepen.io/edgarchen-the-decoder/pen/ZEQWNqx.
2. Mouse moves to green area.
3. Reload the page by keyboard shortcut (for example, Fn+F5 on MAC).
4. Wait page loaded.

Expected results

Mouse cursor update correct while moving out green area.

Actual results

The mouse cursor is stuck with the hand cursor when moving out the green area.
(The cursor get back to normal after hovering over the HTML/CSS/JS top panes)

Comment 4

[Edgar Chen [:edgar]]

And Bug 1606707 is another case.

Comment 5

[Edgar Chen [:edgar]]

Attachment: Bug 1635784 - Test to reproduce the bug;

Comment 6

[Edgar Chen [:edgar]]

Attachment: Bug 1635784 - Part 1: IsRemoteTarget should take fission OOP iframe into account;

And add IsTopLevelRemoteTarget for the original usage.

Comment 7

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66dabca56e2d4596fd8aece7a17baf38967cf4d1

Comment 8

[Edgar Chen [:edgar]]

I also found some bug of mouseenter/mouseleave on nested iframe cases, for example, a.com contains an iframe b.com that also contains an iframe c.com,

• c.com could possibly know nothing about mouse exit in some cases.
• b.com could possibly know nothing about mouse enter in some cases.

Comment 9

[Edgar Chen [:edgar]]

Attachment: Bug 1635784 - Part 2: Do not update cursor if mouse is over a remote target in content process;

Content process could also contain a remote target after fission, we need to do
same check in content process, like what we do in parent process for e10s.

Depends on D79441

Comment 10

[Edgar Chen [:edgar]]

Attachment: Bug 1635784 - Part 3: Make cached cursor consistent between parent and content process;

Content process won't try to send IPC to parent process to update cursor if the
cursor isn't changed, and assume parent process would update cursor properly as
soon as the cursor re-enter to a remote target.

But BrowserParent cached the cursor information for a remote target only when the
remote target is allowed to update the cursor. It could possible that parent use
a stale cursor for a remote target and stuck in that state untill content
process tries to update cursor again.

Comment 11

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4359a683b3a6907baab8674998021645579d64b

Comment 12

[Edgar Chen [:edgar]]

Attachment: Bug 1635784 - Part 4: Should always allow a remote target to update cursor when mouse is over it;

regardless of what the previous remote target is.

Comment 13

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cafc3de43e046fcae63d110e46624fb6828e567

Comment 14

[Edgar Chen [:edgar]]

Attachment: Bug 1635784 - Part 5: Rename mTabSetsCursor;

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b437c79f0bf
Part 1: IsRemoteTarget should take fission OOP iframe into account; r=smaug
https://hg.mozilla.org/integration/autoland/rev/305f34561268
Part 2: Do not update cursor if mouse is over a remote target in content process; r=smaug
https://hg.mozilla.org/integration/autoland/rev/52f5b4638e3d
Part 3: Make cached cursor consistent between parent and content process; r=smaug
https://hg.mozilla.org/integration/autoland/rev/a59cc42cedc4
Part 4: Should always allow a remote target to update cursor when mouse is over it; r=smaug
https://hg.mozilla.org/integration/autoland/rev/aadf9c693336
Part 5: Rename mTabSetsCursor; r=smaug

Comment 16

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/7b437c79f0bf
https://hg.mozilla.org/mozilla-central/rev/305f34561268
https://hg.mozilla.org/mozilla-central/rev/52f5b4638e3d
https://hg.mozilla.org/mozilla-central/rev/a59cc42cedc4
https://hg.mozilla.org/mozilla-central/rev/aadf9c693336

Comment 18

[Julien Cristau [:jcristau] (back August 18)]

From bug 1650657 we may want part of this on esr78.

Comment 19

[Bogdan Maris, Desktop Test Engineering]

Verified this is fixed on latest Nightly 80.0a1 and Nightly 79.0a1 after the fix has landed across platforms (macOS 10.15, Windows 10 64bit and Ubuntu 18.04 64bit).

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Edgar, can you please nominate the relevant patches needed to fix bug 1650657 on ESR78?

Comment 21

[Edgar Chen [:edgar]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #20)

Edgar, can you please nominate the relevant patches needed to fix bug 1650657 on ESR78?

Sure, I would like to verify ESR78 first before nominating.

Comment 22

[Edgar Chen [:edgar]]

Comment on attachment 9157261 [details]
Bug 1635784 - Part 3: Make cached cursor consistent between parent and content process;

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is to fix bug 1650657 which is a regression of bug 1607375.
• User impact if declined: Mouse cursor might get "stuck" under some circumstances, e.g. closing tab via Gesturefy addon then moving the mouse quickly. But could be recovery by moving the mouse out of web content.
• Fix Landed on Version: 79
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only affects the mouse cursor updating and has landed and verified on 79, I have also verified locally on 78esr.
• String or UUID changes made by this patch: None

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9157261 [details]
Bug 1635784 - Part 3: Make cached cursor consistent between parent and content process;

Fixes bug 1650657. Approved for 78.1esr.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/d38354ff64b2
https://hg.mozilla.org/releases/mozilla-esr78/rev/3dde25d513dd