Mouse cursor does not disappear in html5 fullscreen video on Windows 10

VERIFIED FIXED in Firefox 60

Status

()

defect
P1
normal
VERIFIED FIXED
Last year
Last year

People

(Reporter: jimm, Assigned: agashlin)

Tracking

({regression})

44 Branch
mozilla61
x86_64
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox58 wontfix, firefox59 wontfix, firefox60 verified, firefox61 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

STR:

1) play a webm video
2) click fullscreen
3) wait for the escape message to hide

result: mouse cursor should be hidden
actual: mouse cursor is visible over the video

reproduces on Win10, does not reproduce on Win7.

This was fixed in bug 1273091 back in 53. Appears to have regressed.
Priority: P1 → P2
A regression window would be helpful here.
On Windows10 x64 ver1709 build16299.248, I can reproduce on ESR52, Firfox53 as well as 54 and later.
And I can also reproduce the problem on the build Nightly 53.0a1 (buildID: 20161227030213) in Bug 1273091 Comment 42.
(So, Bug 1273091 did not fix anything in this case)

STR:
1) play a video or Youtube video
   https://www.youtube.com/watch?v=bHJ2sOTEpvQ
   https://videos.cdn.mozilla.net/uploads/mozillaorg/A%20different%20kind%20of%20browser.webm
   https://videos.cdn.mozilla.net/uploads/mozillaorg/A%20different%20kind%20of%20browser.mp4
   https://videos.cdn.mozilla.net/uploads/mozillaorg/Firefox_global_final.theora.ogv

2) click fullscreen
3) wait for the escape message to hide, do not move mouse after step 2 click.


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0288a0a7003ffba272dc8040567e225d7115d9bc&tochange=515efe9f75420b46a1803c1cbc3176a446d101dd

Suspect: Bug 506815
Blocks: 506815
Bug 506815, the gift that keeps on giving!
Flags: needinfo?(rares.bologa) → needinfo?(chutten)
If bug 1273091 fixed it, bug 506815 wasn't what regressed it, right?
Flags: needinfo?(chutten)
Oh wait, just noticed in comment#2 that these particular STR weren't addressed by bug 1273091
dparks is away for another few days, but I think a ni? is appropriate to sit that long :)

I'm very interested to hear any theories about why bug 1273091's fix for Windows7 didn't fix it in Windows 10.
Flags: needinfo?(davidp99)
Adam is picking this up.
Assignee: nobody → agashlin
Flags: needinfo?(davidp99)
Priority: P2 → P1
Duplicate of this bug: 1270776
The root of the problem seems to be that we get a WM_MOUSELEAVE when resizing on Windows 10 (and not Win 7), but it comes too late for the fix in bug 1273091 (which inserts a synthetic eMouseEnterIntoWidget) to work around it.

I'm inclined to fix this by not ignoring the WM_MOUSEMOVE that we get following the WM_MOUSELEAVE, instead letting it generate a eMouseEnterIntoWidget naturally at [1]. The most straightforward way of doing this is with the attached patch; this seems to fix the issue.

Currently running through try at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c44678e476d898a0eb45a203ce7b44622a5b228

[1] https://searchfox.org/mozilla-central/source/widget/windows/nsWindow.cpp#4648
A side effect of the patch in comment 9 is that when a popup appears whatever is under the cursor immediately becomes selected as if the mouse had moved there; we probably don't want this behavior (and indeed this may be the whole reason why the check I'm amending, "Suppress moves caused by widget creation", is there). At the very least it breaks a test involving the search popup [1]. I probably should only be doing a eMouseEnterIntoWidget in this case rather than let it fall all the way through and look like a real mouse move, but now I'm worried that even that might be too general.

I'm going to look back at some older Windows 10 builds to see if I can tell why the original fix in bug 1273091 no longer works.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c44678e476d898a0eb45a203ce7b44622a5b228&selectedJob=164753981
I found that in Windows 10 build 15063 we get a WM_MOUSELEAVE early, before any events related to resizing the window, but in 16299 this message comes late, after all the related events. We really don't want to get it at all, so the attached patch cancels TME_LEAVE tracking just before we go into fullscreen. I also backed out the bug 1273091 fix as it is now no longer needed. TME_LEAVE will be reenabled on the first mouse move inside the window.

I think this should work on both new and old builds of Windows 10, and on Windows 7, without changing any of the other semantics around the mouse. Currently running through try at:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0194fa5a6be0badec208628043316c64e5cb1ad5
Attachment #8954600 - Attachment is obsolete: true
Attachment #8956307 - Flags: review?(davidp99)
Comment on attachment 8956307 [details] [diff] [review]
Cancel TME_LEAVE before going fullscreen

This looks good.

nsWindow::MakeFullScreen has guards for re-entry (ie it checks if we are requesting fullscreen when already in fullscreen) but this patch would first cancel mouse leave tracking.  Its almost a nit but I think the cancel should be after this part of the function:

>  if (aFullScreen) {
>    if (mSizeMode == nsSizeMode_Fullscreen)
>       return NS_OK;

r+ with that change.
Attachment #8956307 - Flags: review?(davidp99) → review+
Comment on attachment 8956307 [details] [diff] [review]
Cancel TME_LEAVE before going fullscreen

I noticed that this does not work properly on older Win 10 builds (15063), and I now realize where the WM_MOUSELEAVE is coming from and why it is showing up in different places: it is caused by the fullscreen transition window (the black screen). When that window animates in it must be taking the mouse away from us, but Windows realizes that at different times in different versions; removing ::AnimateWindow [1] makes the issue go away.

With this in mind, we probably want to turn off tracking until the transition is finished. I'm not sure how to get that event to the right place yet.

[1] https://searchfox.org/mozilla-central/rev/56274d0a016a6833e5da7770ce70580b6f5abb21/widget/windows/nsWindow.cpp#3294
Attachment #8956307 - Attachment is obsolete: true
Duplicate of this bug: 1446664
This bug also happens, fairly rarely, on current Windows 7. I hadn't seen it before as I wasn't running a Win 7 VM with desktop compositing enabled, so the fade transition wasn't happening. Whereas on old Win 10 builds WM_MOUSELEAVE comes during the AnimateWindow call, and on newer Win 10 builds it comes after that call, on Win 7 it seems to randomly be during or after. As it seems to be a race condition I'm sure a lot goes into the exact timing.

I've tried a few things but I think the least hacky approach is essentially what I did in comment 9: Don't ignore the first WM_MOUSEMOVE after WM_MOUSELEAVE, which is how Windows tells us that the mouse is back over the main window. In order to avoid getting unwanted mouse enters as described in comment 10, I'm doing this by clearing sLastMouseMovePoint when we get a WM_MOUSELEAVE and the mouse is over the transition window.
Attachment #8964457 - Flags: review?(davidp99)
The patch looks ok to me but I don't know enough about the FullScreenTransitionTask to say for sure that clearing the mTransitionWnd as part of the FullScreenTransitionTask state machine, as you do, is always safe.  It _looks_ like the state machine may not always get to the eEnd state, which would leak.  But I'm not sure.  

The code that runs the state machine, FullScreenTransitionTask::Observer::Observe [1], looks like it was introduced in bug 1160014.  Its hard to track down because the file was deleted but the code is in the bug [2], although I note that the first version doesn't include the eEnd state (that must have gone in later).  So NI to :xidorn, hoping you know the answer to this: can we count on all of the state transitions always being run in the FullScreenTransitionTask?  Or is the code in Observe intended to sometimes abort the state machine early?

I'll r+ if the state machine is safe.

[1] https://searchfox.org/mozilla-central/rev/a0665934fa05158a5a943d4c8b277465910c029c/dom/base/nsGlobalWindowOuter.cpp#4189
[2] https://bug1160014.bmoattachments.org/attachment.cgi?id=8631328
Flags: needinfo?(xidorn+moz)
Thanks! I'll be glad to have Xidorn have a look, too.

The eEnd state was added in bug 1267530; for future reference here's the annotated last version of nsGlobalWindow.cpp:
https://hg.mozilla.org/mozilla-central/annotate/74086caf9463/dom/base/nsGlobalWindow.cpp
(In reply to David Parks (dparks) [:handyman] from comment #17)
> NI to :xidorn, hoping you know the answer to this: can we count on all of
> the state transitions always being run in the FullScreenTransitionTask?  Or
> is the code in Observe intended to sometimes abort the state machine early?

For this question, I think the answer is yes, we can count on all of the states would be run, unless the window widget itself goes away, in which case FullscreenTransitionTask::Run would early return without doing anything further. I guess that's not a case you are worried about?

FullscreenTransitionTask observes two events, one is the painting event, the other is a timer event, whichever happens first would proceed the state machine, so as soon as timer is still ticking, it should always allow us to enter the next stage.
Flags: needinfo?(xidorn+moz)
Comment on attachment 8964457 [details]
Bug 1437941: Don't ignore mousemove after fullscreen transition

Thanks - that's the answer I was looking for.
Attachment #8964457 - Flags: review?(davidp99) → review+
oops, :handyman could you flip that to r+ on the reviewboard side as well?
Flags: needinfo?(davidp99)
Comment on attachment 8964457 [details]
Bug 1437941: Don't ignore mousemove after fullscreen transition

https://reviewboard.mozilla.org/r/233196/#review239432
that was weird.
Flags: needinfo?(davidp99)
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d725bf734be
Don't ignore mousemove after fullscreen transition r=handyman
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1d725bf734be
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Fixed on both win7 and win10.
Status: RESOLVED → VERIFIED
Hey Adam, do you feel comfortable uplifting to 60? We'd have about a month on beta before it ships.
Flags: needinfo?(agashlin)
Comment on attachment 8964457 [details]
Bug 1437941: Don't ignore mousemove after fullscreen transition

Approval Request Comment
[Feature/Bug causing the regression]: originally regressed by bug 506815, exposed again (after fix in bug 1273091) due to a timing change in Windows 10 Fall Creators Update
[User impact if declined]: User has to move mouse before cursor will disappear after switching to fullscreen video, irritating
[Is this code covered by automated tests?]: Yes, but there is no automated test for this specific behavior (cursor visibility)
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: Low complexity, the fix only applies when a fullscreen transition occurs, and it should only affect mouseenter/mouseleave behavior immediately surrounding that transition
[String changes made/needed]: none
Flags: needinfo?(agashlin)
Attachment #8964457 - Flags: approval-mozilla-beta?
Comment on attachment 8964457 [details]
Bug 1437941: Don't ignore mousemove after fullscreen transition

this feels borderline for uplift at this stage, but let's give it a go.  approved for 60.0b12
Attachment #8964457 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Managed to reproduce this bug using the STR from comment 0, on an affected Nightly build 61.0a1 (2018-03-29) with the following videos:
- https://www.youtube.com/watch?v=bHJ2sOTEpvQ
- http://dl3.webmfiles.org/big-buck-bunny_trailer.webm

Verified fixed under Windows 10 x64, on latest Nightly 61.0a1 (2018-04-13) and Beta 60.0b12 (20180412172954)
Don't know if this is related but on Ubuntu with Firefox version quantum 60.0.2 64 bit the mouse disappears when a video triggers full screen but if you enable full screen with f11 the mouse does not disappear. So when using YouTube.com/tv in f11 full screen you have to move the mouse of the screen but if you use YouTube not on tv mode and click on the full screen icon on the video the mouse goes away
(In reply to xraya4t from comment #33)
> Don't know if this is related but on Ubuntu with Firefox version quantum
> 60.0.2 64 bit the mouse disappears when a video triggers full screen but if
> you enable full screen with f11 the mouse does not disappear. So when using
> YouTube.com/tv in f11 full screen you have to move the mouse of the screen
> but if you use YouTube not on tv mode and click on the full screen icon on
> the video the mouse goes away

Hi xraya4t, thanks for the report. I don't think this is the same issue. I have seen this in Windows on both Firefox and Chrome, the mouse cursor never disappears on youtube.com/tv. I think that this is just how that site is designed.
You need to log in before you can comment on or make changes to this bug.