Closed Bug 590715 Opened 10 years ago Closed 10 years ago

Fullscreen video for Fennec

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file, 2 obsolete files)

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.)
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0b2+
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?
Assignee: nobody → mbrubeck
Let's get video platform feedback
Depends on: 602976
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!
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.
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.
Attached patch WIP (obsolete) — Splinter Review
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.
Status: NEW → ASSIGNED
Depends on: 465839
Attached patch WIP 2 (obsolete) — Splinter Review
Forgot to hg add the HTML file.
Attachment #482713 - Attachment is obsolete: true
Attached patch patchSplinter Review
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.
Attachment #482734 - Attachment is obsolete: true
Attachment #483551 - Flags: review?(mark.finkle)
Attachment #483551 - Flags: feedback?(wjohnston)
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.
Attachment #483551 - Flags: review?(mark.finkle) → review+
(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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attachment #483551 - Flags: feedback?(wjohnston)
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
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Flags: in-litmus? → in-litmus?(tchung)
You need to log in before you can comment on or make changes to this bug.