782008 - Page body behind full screen HTML5 video is interactive with long press

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

1) Open Firefox
2) Go to http://carmendesign.com/code/video_for_everybody/test.html
3) Once the video is partially loaded, long press the video and select "Full screen" on the resulting context menu.
4) Long press the space around the video (it may be easier if you hold it in portrait mode).

Expected: Nothing happens (or the "Exit Full Screen" context menu appears)
Actual: Sometimes a context menu will pop up for various links and images on the page where this video came from.

Tested Galaxy Nexus, Android 4.0.4.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Sorry, bad link: http://camendesign.com/code/video_for_everybody/test.html

Comment 3

[:Margaret Leibovic]

Attachment: quick fix

I logged rootElement in _sendToContent, and when I'm running into this bug, the problem is that rootElement isn't the video element. It seems like the real bug here is that we're not getting back the video element from elementFromPoint, but since that sounds tricky to investiagte, this patch is a simple fix to avoid showing a context menu when we're in full screen mode.

My one concern with this is that we'll need to change our approach if we ever want to show a context menu on full screen video, or if we add support for HTML5 context menus, since a webpage may want to add its own video context menu.

Comment 4

[:Margaret Leibovic]

Wes, review ping?

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 653597 [details] [diff] [review]
quick fix

Review of attachment 653597 [details] [diff] [review]:
-----------------------------------------------------------------

I... don't like doing this. Lets try to get someone to figure out why DOMWindowUtils.elementFromPoint isn't working in fullscreen. Maybe even clone this bug and file something in layout.

Comment 6

[Johnny Stenback (:jst)]

I agree with wesj here, this seems like us patching around a bug that we'll likely run into in other frontends too (metro, b2g, etc). I think we should either fix this in nsDOMWindowUtils, nsDocument::ElementFromPointHelper(), or in layout. Cc:ing cpearce and roc for further clarification as to where a fix would make most sense here.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

This looks like bug 790850 but on fennec instead of b2g. Anthony Jones is currently looking at fixing this bug on B2G, the same fix would hopefully fix things on fennec too...

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think he's had time to look at it on B2G ... I suggest you take this bug and try to fix it first, perhaps fixing the B2G bug (though I'm doubtful this is the same bug as that).

The nsDisplayBackground of the fullscreen video element should be covering the entire screen and catching all events here, so elementFromPoint should always be returning the video element. It would be pretty easy to instrument a build to dump the display list built indirectly by elementFromPoint.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Wild stab in the dark --- Jonathan, could you look into this B2G bug? Someone who understands display lists and event handling would have a head start :-)

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Er, this is a Firefox for Android bug, not a B2G bug.

Comment 11

[Nick Cameron [:nrc]]

I've started looking at this, but between having to download a new Android SDK and long, dodgy builds I haven't got much further than reading the appropriate bits of code.

jwatt - if you haven't started looking at this yet I'll take it.

Comment 12

[Jonathan Watt [:jwatt]]

Feel free. I've only just this morning got past the build errors I was encountering and successfully got a fennec build.

Comment 13

[Nick Cameron [:nrc]]

Attachment: display list dump

This is the display list dump from nsLayoutUtils::GetFrameForArea (called by nsDocument::ElementFromPoint). I'm not sure why there are 5 calls, but this is really only from the long touch, I couldn't narrow it down any further. I suppose we might call GetFrameForArea from other places and/or we could test at the beginning, end, and possibly middle of the touch. Nothing jumps out at me yet.

Comment 15

[Nick Cameron [:nrc]]

Handing off - I don't have much in the way of leads here, that display list used for hit detection looks fine to me, and the top element is getting picked as expected. I was going to start looking into the JS side of things:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1452
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1490
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#4171

But I suspect the problem is in C++ and I just can't see it.

Comment 16

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.