1681692 - In certain case, cursor is not hidden correctly when fission is enabled

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Xidorn Quan [:xidorn] UTC+10]

I observed this on https://ani.gamer.com.tw/ which is a video streaming service only available on Taiwan.

When I watch videos in fullscreen, the cursor would not be correctly hidden. This is fixed when opening in a window with fission disabled.

I inspected the code, and found that the website correctly sets cursor: none on the fullscreen element, which is an ancestor of the <video>, but it doesn't work. If I manually set cursor: none on the <video> element, it's correctly hidden.

I failed to construct a minimal testcase from it, so I have no idea what's happening here...

Comment 1

[Emilio Cobos Álvarez (:emilio)]

May be one of the pre-existing hit-testing bugs with fission (see bug 1519412 and co.)

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Xidorn, if you have time to generate a simple test case, it would be quite helpful, I couldn't play videos on the site from Japan as you are saying unfortunately. Thanks!

Comment 3

[Chris Peterson [:cpeterson]]

Xidorn, can you please save a copy of the video player page's HTML (Web Page Complete) and share it on this bug? This will help the Fission team see the code until we can get VPN access into Taiwan.

This bug should only happen in Fission if the video player page is using an OOP iframe. Perhaps the videos on this page are embedded from another site?

Comment 4

[Edgar Chen [:edgar] (PTO untils 8/11, slow response)]

I could reproduce this issue, but not 100% though.
The video isn't embedded from another site, but there are some ad iframe.
I tried to open Inspector to check whether cursor: none is set correctly , but the cursor is correctly hidden after I open the Inspector.

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

In my case, switching window back and forth would hide the cursor, but if I move it a bit then it keeps being there again even though I can see the style change from inspector (in a different monitor).

Comment 6

[Hiroyuki Ikezoe (:hiro)]

So the video is a position:absolute element or some such? I mean it's out-of-flow thingie? Then it's possible that it's another variant of bug 1676466.

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Attachment: testcase

Okay I have a reproducible testcase.

Comment 4 inspired me, so I had another attempt. It seems that there are three <iframe>s on the page, and if I remove the one on the bottom (for Facebook comments), then the cursor would be correctly hidden.

So I add an iframe into the testcase, and it works!

Steps to reproduce:

1. open the page
2. click fullscreen
3. move cursor in the page

Expected behavior (when fission is disabled):
the cursor should hide and show every two seconds

Actual behavior (when fission is enabled):
the cursor sometimes doesn't change, most of time keep being there

Comment 8

[Edgar Chen [:edgar] (PTO untils 8/11, slow response)]

Thanks for the testcase!

And interesting, I got the same result with Inspector as comment #4.

• If I open inspector on the same window and watch the element that adds cursor: none, I could not reproduce the issue, cursor hide and show correctly.
• But if I open inspector on a separate window (in a different monitor). , then the issue happens.

I will take a look.

Comment 9

[Edgar Chen [:edgar] (PTO untils 8/11, slow response)]

Attachment: testcase2.html

Comment 10

[Edgar Chen [:edgar] (PTO untils 8/11, slow response)]

The cursor isn't updated correctly because the synthesized mouse event for cursor updating is dispatched to the OOP iframe here instead.

Comment 11

[Edgar Chen [:edgar] (PTO untils 8/11, slow response)]

(In reply to Edgar Chen [:edgar] from comment #10)

The cursor isn't updated correctly because the synthesized mouse event for cursor updating is dispatched to the OOP iframe here instead.

It is because the FindViewContaining returns OOP iframe's view here, :tnikkel, :botond, is this something expected? In the testcase page, FindViewContaining returns iframe's view even if the mouse location isn't over the iframe.

Comment 12

[Timothy Nikkel (:tnikkel)]

That seems wrong, I'll look into it.

Comment 13

[Neha Kochar [:neha]]

Assigning to Timothy based on comment 12. Thanks Timothy for looking into this!

Comment 14

[Neha Kochar [:neha]]

This isn't critical enough to block M6c - pushing this out to M7 for Fission Beta.

Comment 15

[Timothy Nikkel (:tnikkel)]

The problem is that we incorrectly think we are crossingTheZoomBoundary so we skip the bounds check when moving from the root view of the root content document to a child view. Easy enough to fix. Need to write a test, maybe something from test_hover.html but in an oopif (necessary otherwise we dispatch the event to the correct process even if we find the wrong view).

Comment 16

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1681692. In FindViewContaining we are only crossingZoomBoundary if we start in visual coordinates. r?botond

In the most common case the first call to FindViewContaining has aRelativeToView = aView = the root view of the root content document, and we start in visual coords. Then we convert to layout coords and call FindViewContaining recusively with aRelativeToView = the root view again, aView = a child of the root view and working in layout coords. And we still satisfy the conditions to set crossingZoomBoundary to true, which is incorrect because we already crossed. We need to check that we are also in visual coords.

Comment 17

[Bill Goldstein [:aja] (UTC-6)]

FWIW, possible test case idea: it's noticeable on pages like hg pushlog that have hover state for table rows.

Comment 18

[Neha Kochar [:neha]]

Timothy, are you able to write a test for it and close the work for this issue?

Comment 19

[Timothy Nikkel (:tnikkel)]

I will be able to at some point, but I have a lot of MR1 work that needs to be done.

Comment 20

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

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:tnikkel, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 21

[Timothy Nikkel (:tnikkel)]

Patch needs a test. Busy with mr1 work, can't get to it yet.

Comment 22

[Chris Peterson [:cpeterson]]

(In reply to Timothy Nikkel (:tnikkel) from comment #21)

Patch needs a test. Busy with mr1 work, can't get to it yet.

:tnikkel, do you think you will be able to write the test and land this patch by mid-June?

The Fission team is currently tracking this bug as a blocker for our Fission M7a milestone to launch a big Fission Beta experiment in 91 Beta (end of June). If not, I will need to find another engineer so this bug doesn't delay our M7a experiment.

Comment 23

[Timothy Nikkel (:tnikkel)]

Yes, mr1 will be done before then.

Comment 24

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

Reminder that the fission beta experiment will start soon, and 90 is now on beta.

Comment 25

[Chris Peterson [:cpeterson]]

Timothy, since this bug is just blocked waiting for a new test, can we land your fix now (for Fission Milestone M7a) with a follow-up bug for the test (in Fission Milestone M8)?

Comment 26

[Frank Griffith Jr]

Tim, re-posting the NI as we didn't see this closed out, or the test only bug created. Thanks

Comment 27

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1681692. Add test. r?botond,hiro

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c675319da0d
In FindViewContaining we are only crossingZoomBoundary if we start in visual coordinates. r=botond
https://hg.mozilla.org/integration/autoland/rev/912f065c513f
Add test. r=hiro

Comment 29

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6c675319da0d
https://hg.mozilla.org/mozilla-central/rev/912f065c513f

Comment 30

[Xidorn Quan [:xidorn] UTC+10]

I can confirm that this has been fixed on the website I originally found this issue.