590715 - Fullscreen video for Fennec

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Add a context menu item to open <video> elements in full-screen mode.  (We discussed also using double-tap to enter/leave full-screen mode, but decided to use the context menu only, for now.)

The fullscreen mode could be implemented similar to Firefox, using mozLoadFrom() inside of a new HTML overlay:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#807
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/fullscreen-video.xhtml

We should consider how this will interact with video controls (bug 465839).  For example, we might want to display the control overlay only in fullscreen mode.  Also, should we force landscape orientation when playing full-screen landscape-format videos?  (Native video players often do this.)

Comment 1

[Matt Brubeck (:mbrubeck)]

Firefox creates a new <video> element in a chrome dialog, then uses nsIDOMHTMLMediaElement.mozLoadFrom to make the new element share data with the old one (so that nothing will be downloaded twice).

To do something similar in Fennec with e10s, we need to answer these questions:

1) We can't call mozLoadFrom in chrome because we can't get a reference to the original DOM element from the child process.  Can we modify mozLoadFrom to work cross-process?

2) Is it okay to play videos in the parent process, or should we display the fullscreen video within a remote browser?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's get video platform feedback

Comment 3

[Daniel Bruce Lynes]

I think it should automatically switch into full screen, similar to how the HTC Flash player does it.  I'm using it on the HTC Legend, fwiw.  Also, congratulations on getting the ARMv6 alpha to work on the Legend, now!

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Stuart had an idea where we use a <browser remote="true"> to display the fullscreen video. Since the video would be in a remote browser, we should have access to nsIDOMHTMLMediaElement.mozLoadFrom, since it's in the same process.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We definitely want to decode video in content processes, for a variety of reasons.  Bug 598868 covers making that fast, though that's mostly orthogonal to how we paint to the full screen.

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: WIP

This proof of concept patch implements the suggestion in comment 4, and plays the video in a full-screen <browser remote>.  It's working fairly well.

Still to do:
* Need to handle alerts and notifications popping up over the video.
* Make it play nicely with the controls from bug 465839.
* Put the top-level window in full screen mode (depends on bug 602976).
* Clean up code for checkin.

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: WIP 2

Forgot to hg add the HTML file.

Comment 8

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This works on desktop and on device.

There are some parts I'd kind of like to fix, possibly in followups:
* I copied the timeout and behavior from the video controls in bug 483024, to try and make the close button appear and disappear at the same time.  This is hacky and fragile.
* video.jsm is just a glorified global variable for passing an element from one browser to another.  Is there a better way to do this?
* Since we don't allow panning in fullscreen mode, we could let the user drag the slider.  We could hook up a customDragger to generate mousemove events.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 483551 [details] [diff] [review]
patch

* Remove dumps before landing.
* I can't think of a way around video.jsm, unless we can get "handles" to work. You might be able to send the DOM node via an observer notification, which is caught by the fullscreen-video.xhtml frame script, then injected into the DOM. Maybe we should embrace the video.jsm approach and make it generic, not video specific.

Comment 10

[Matt Brubeck (:mbrubeck)]

(In reply to comment #9)
> * Remove dumps before landing.

Done, and pushed: http://hg.mozilla.org/mobile-browser/rev/6e2f643b6a0a

> * I can't think of a way around video.jsm, unless we can get "handles" to work.
> You might be able to send the DOM node via an observer notification, which is
> caught by the fullscreen-video.xhtml frame script, then injected into the DOM.
> Maybe we should embrace the video.jsm approach and make it generic, not video
> specific.

I'll land this for b2, and file followups for some of the notes above.

Notes for testing:

* This won't be fully usable until our new video controls land (bug 465839).
* You can exit fullscreen by pressing escape (desktop), pressing the hardware "back" button (Android), or by tapping the "X" that appears when you touch the screen (all platforms).
* The "X" should fade out after 5 seconds (just like the controls will after bug 465839 lands).
* The window won't be fullscreen on Android until bug 602976 lands.  Until then, the system alert bar will still be visible.

Comment 11

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (maemol Linux armv7l; rv:2.0b7pre) Gecko/20101029 Firefox/4.0b8pre Fennec/4.0b2

Mozilla/5.0 (android Linux armv7l; rv:2.0b7pre) Gecko/20101029 Firefox/4.0b8pre Fennec/4.0b2