Last Comment Bug 453063 - Support for fullscreen video playback
: Support for fullscreen video playback
Status: VERIFIED FIXED
: verified1.9.2
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: P1 enhancement with 29 votes (vote)
: Firefox 3.7a1
Assigned To: Dão Gottwald [:dao]
:
Mentors:
: 499428 (view as bug list)
Depends on: 520806 370857 513144 517870 520160 520600 520805 521129 811261 830660 968526 1064787
Blocks: 531576
  Show dependency treegraph
 
Reported: 2008-08-31 14:12 PDT by Justin Dolske [:Dolske]
Modified: 2015-04-13 06:04 PDT (History)
54 users (show)
mbeltzner: blocking‑firefox3.6+
chung808: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed


Attachments
basic implementation (9.56 KB, patch)
2009-07-03 15:50 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Review
basic implementation (13.27 KB, patch)
2009-07-10 00:28 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Review
patch (21.38 KB, patch)
2009-09-21 04:31 PDT, Dão Gottwald [:dao]
dolske: review+
Details | Diff | Review
review comments addressed (21.81 KB, patch)
2009-09-23 01:36 PDT, Dão Gottwald [:dao]
jboriss: ui‑review+
Details | Diff | Review

Description Justin Dolske [:Dolske] 2008-08-31 14:12:40 PDT
It should be possible to play videos in a full-screen mode. Every stand-alone media player supports this, and most Flash-based players do too.

The HTML5 spec warns against allowing content to invoke full-screen mode (security), but there should be a way for the browser chrome to invoke it. EG, from a button on the video control overlay, a context menu, or whatever.
Comment 1 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-13 20:24:18 PST
Not for 3.1 I think. Maybe the following release.
Comment 2 pd 2008-11-13 20:31:03 PST
Hooray, another kick in the **** for open standards. Why oh why do I bother. Adobe Flash, here I come!
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2008-11-13 20:33:07 PST
It's just too late in the 3.1 cycle for new features like this. There are a lot of other video-related bugs that are more important that we need to focus on.
Comment 4 pd 2008-11-13 20:46:38 PST
I asked Chris Double on IRC whether the seemingly long (to my inexperienced eye) list of video / audio bugs was a concern to him. He said it was not. I guess that does not necessarily mean there is time for 'new' features but this is still disappointing.

Realistically though, I guess the most important thing for 3.1 is stability, reliability and performance of video/audio. Open standards and native support for multimedia is already so far behind Flash that I guess it's too much to expect a multi-million dollar organisation like Mozilla to catch up in a mere two years (sarcasm intended).

At least tell me the DirectShow support is going to land in 3.1? At least then the most popular codec on the net - XviD - can be supported and I guess resolutions can at least be set to max out the browser window. At least then, inadvertently and in spite of joke that is web standards and the mozilla priorities, open video (XviD) might have a chance of immediately competing with Flash players given how ubiquitous XviD is (even Microsloth are supporting it in Windows 7) and OGG is absolutely not.
Comment 5 Robert Kaiser (not working on stability any more) 2008-11-14 03:57:21 PST
Please stay on-topic in bug reports. This report is about full-screen support so only comment on how to help implementing this, everything else should be taken to newsgroups. That said, I didn't know that MPEG4 (including XviD) was patent-free and therefore really open nowadays and that DirectShow was an open cross-platform standard as you imply. Still, all that doesn't have anything to do with full screen support, which is tricky in its own way across platforms.
Comment 6 Fritz 2008-12-02 04:54:39 PST
I think the quickest way to implement full screen browsing is to have on the context menu an option to "open" the file on your computer based on your MIME type prefs.  So, right click on a video element, it will open in VLC or MPC and then I full screen it there.  We have send video as it is... not sure why we can't have open... although, I suppose save video->open works... the difference being that open would just keep the video in its temporary location and save a bit of hassle.

I must say with a lot of these more complex video requests that involve increasing user control I am wary about them coming into Firefox... not because I don't want them but because I want to be sure that they can be done better by Firefox rather than VLC/MPC.  We need native support for video so you don't need plug in to view it on a webpage... but, I think once you're going into full screen / detach that you're doing things with the video that can/should be done by the player of your choice and not by Firefox.  Firefox should facilitate quick loading into said player but it would take years fore Firefox to implement these features that work perfectly fine in VLC/MPC and don't add the benefit of integrating with webpage material.
Comment 7 Biju 2008-12-20 22:44:32 PST
(In reply to comment #6)
> I think the quickest way to implement full screen browsing is to have on the
> context menu an option to "open" the file on your computer based on your MIME
> type prefs. 

created bug 470627
Comment 8 Justin Dolske [:Dolske] 2008-12-20 22:52:39 PST
*** Bug 470628 has been marked as a duplicate of this bug. ***
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-04-02 01:40:30 PDT
Justin, I actually think it would be easy to implement this as a context menu item, much like "View Video" except that the new window is made fullscreen, we give it a black background, no margins, and set the video element in the window to width:100% height:100%. Any key event or mouse-down event could close the window. Would be cool to do...
Comment 10 Justin Dolske [:Dolske] 2009-04-02 02:04:38 PDT
Yeah, I was considering doing that as an extension, since:
 * We can't do full-screen on OS X, afaik
 * I was concerned about there being performance issues
 * It's a really useful example of an extension, err, extending <video> UI
 * Doing it right would mean having a different control design in full-screen

It sounds like there have been some performance improvements recently, so perhaps software scaling to 1920x1200 (or whatever one's desktop happens to be) isn't going to be a strain for most systems?

I think the controls will be in good shape for beta 3, so I can look after that to see if it makes sense to try and slip this in by RC1, or only do it as an addon.
Comment 11 cajbir (:cajbir) 2009-04-02 02:16:10 PDT
> perhaps software scaling to 1920x1200 
> (or whatever one's desktop happens to be)
> isn't going to be a strain for most systems?

As long as 'most' means at least dual core 2Ghz+. That manages to play back 1080p.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-04-02 02:27:13 PDT
The performance penalty relative to the non-fullscreen case would depend entirely on how fast the scaling operation runs. On Windows, I believe that depends on pixman, and I'm not sure how good that would be. Might be worth trying anyway.
Comment 13 Chris Pearce (:cpearce) 2009-04-02 13:52:35 PDT
When I was working on the DirectShow backend, I used DirectX to scale the video frame, as the gfxImageSurface scale operations (in nsMediaDecoder::Paint()) were really killing me.
Comment 14 Aaron Train [:aaronmt] 2009-06-19 20:24:31 PDT
*** Bug 499428 has been marked as a duplicate of this bug. ***
Comment 15 cajbir (:cajbir) 2009-06-19 20:36:45 PDT
tinyvid.tv gets a lot of comments from people testing video for Firefox. Almost every comment has 'needs fullscreen option'. Personally this surprises me since I don't think I've ever used fullscreen at YouTube but it's obvious to me that it is very important to other video users.
Comment 16 d 2009-06-21 05:37:27 PDT
Marking as wanted for 1.9.2 as that is what seems to be possible.
Comment 17 robinvos 2009-06-23 15:06:29 PDT
I'd love to see full-screen supported. Or at least an option to make the video twice as large. It was the very first thing I noticed when testing the new Firefox 3.5 plugin-less video support.
Comment 18 Asa Dotzler [:asa] 2009-06-23 15:27:51 PDT
I expect that some of this will be ameliorated by the various web UI toolkits which will, hopefully, implement nice video player controls that will include nifty features like drag-to-resize and "big/medium/small" resize buttons, etc. 

Still, there aren't any serious video sites that don't have a full-screen mode in their players. At a minimum, I think we could do some kind of "full-browser-window" mode where we push the video out to the edges of the browser window (as large as possible while preserving aspect ratio)
Comment 19 Alvaro 2009-06-30 11:43:42 PDT
I strongly support the addition of "full-screen" video. I think most of the technology is already here: you can zoom a video with Ctrl-'+' or Ctrl-mousewheel and it plays smoothly, you can also press F11 for fullscreen browsing (no menubar, no taskbar). So, it seems it's a matter of implementing the control.

Hint: Google Chrome scales video with no interpolation (worse quality but less CPU needed). Could be a fallback mode for slower computers which would drop frames in fullscreen.
Comment 20 Mike.lifeguard 2009-06-30 21:04:53 PDT
(In reply to comment #19)
> I strongly support the addition of "full-screen" video.

Pro-tip: You don't need to say "me too" because the bug is already open. Unless your comment furthers fixing the issue it really doesn't help the sound-to-noise ratio.
Comment 21 Dão Gottwald [:dao] 2009-07-03 05:57:31 PDT
Can somebody give this a try on Mac?
http://en.design-noir.de/mozilla/fullscreen-video/0.5.xpi

Depends on bug 370857, I suppose.
Comment 22 Mark D. 2009-07-03 09:24:49 PDT
I tried it and it works.
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

I noticed the video is re-downloaded and that it's not yet _true_ full screen; but the ESC button works, the aspect ratio remained the same, and the video played smoothly!
Comment 23 Asa Dotzler [:asa] 2009-07-03 12:23:23 PDT
Yes, on mac, this goes nearly fullscreen, but retains the Mac titlebar, feeling a lot like what I suggested as a minimal solution in comment 18.  As noted above, the biggest flaw in the implementation is the restart of the video.
Comment 24 Dão Gottwald [:dao] 2009-07-03 13:53:11 PDT
Chris explained why the video is re-downloaded in bug 462378 comment 14. Directly jumping to the content video's position would depend on that being fixed.
Comment 25 Dão Gottwald [:dao] 2009-07-03 15:50:04 PDT
Created attachment 386769 [details] [diff] [review]
basic implementation
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-07-03 18:18:41 PDT
A hacky way to avoid redownloading the video would be to remove the video element from the old document and put it in the new document, and put it back later. This would break some pages though.
Comment 27 cajbir (:cajbir) 2009-07-04 07:16:01 PDT
I tried the extension from comment 21 on Linux. It works very well!
Comment 28 Justin Dolske [:Dolske] 2009-07-05 19:17:59 PDT
(In reply to comment #21)
> Can somebody give this a try on Mac?
> http://en.design-noir.de/mozilla/fullscreen-video/0.5.xpi

I started some work a week or so ago on a similar idea:

https://people.mozilla.com/~dolske/tmp/fullscreen-0.1.xpi

I used a <panel> instead of just a window, to work around OS X's titlebar problem. And tied it into the video controls (ugly hack!). I didn't pursue it much farther, though, because the performance on OS X was absolutely terrible. On my 1900x1200 monitor + new MacBook Pro, a full screen video would only play at ~3 fps and was basically unwatchable. At roc's suggestion I tried using CSS's "image-rendering: -moz-crisp-edges", but that didn't seem to help much.

Looks like there may be some perf issues with <panel>, because a fullscreen video with your extension seems to have a slightly higher frame rate. But it's still really bad. :-( I'd be curious to know how things fare on Windows and Linux.

My assumption for this bug is that we'd want a "real" fullscreen mode, meaning hardware scaling (as most FS video players do).
Comment 29 Dão Gottwald [:dao] 2009-07-06 00:23:14 PDT
(In reply to comment #28)
> Looks like there may be some perf issues with <panel>, because a fullscreen
> video with your extension seems to have a slightly higher frame rate. But it's
> still really bad. :-( I'd be curious to know how things fare on Windows and
> Linux.

It works well on my run-of-the-mill Atom N270 netbook at 1024x600, except for bug 502440 on Linux.

> My assumption for this bug is that we'd want a "real" fullscreen mode, meaning
> hardware scaling (as most FS video players do).

I think that's on the agenda for 1.9.2.
Comment 30 Biju 2009-07-08 20:49:40 PDT
Tested on Vista and XP, 
Dão's  http://en.design-noir.de/mozilla/fullscreen-video/0.5.xpi
Pros: True Full Screen, escape key

Justin's https://people.mozilla.com/~dolske/tmp/fullscreen-0.1.xpi
Pros: Full screen button, escape key, context menu item

issues:-
on both, original video continues play in background which produce an echo.
full screen always start with current position a zero.
Comment 31 Dão Gottwald [:dao] 2009-07-09 14:00:17 PDT
(In reply to comment #30)
> on both, original video continues play in background which produce an echo.

I don't see this. The implementation is expected to pause the original video, which works for me.
Comment 32 Biju 2009-07-09 19:46:57 PDT
(In reply to comment #31)
> The implementation is expected to pause the original video,

Tested a lot and finally reproduced it.
It was happening because of Justin's extension And my test steps.
steps:-
1. install both extensions
2. go to http://www.double.co.nz/video_test/transformers480.ogg
3. click on Justin's Zoom button
   .... video zoomed, with video at currentTime=0
4. Click play button on the Zoomed video
5. Click anywhere outside of the new zoomed screen
   ... Zoom dismissed
6. Now right click and select "Full Screen" to run Dão's extension.
7. Wait for few seconds.
   user here video from Both extensions.

ie, even though the "panel" dismissed from view, 
it was there in the background, and running the videos.
Comment 33 thehans 2009-07-10 00:03:52 PDT
If anyone is interested, I've created a greasemonkey script which maximizes a video on a page to the size of the firefox window.  It works seamlessly during video playback, without restarting from the beginning.
However, it is still up to the user to set the Firefox main window into fullscreen mode, with F11.

http://userscripts.org/scripts/show/53429
Double click the video element to maximize, and again to restore original size.
Comment 34 Dão Gottwald [:dao] 2009-07-10 00:28:33 PDT
Created attachment 387833 [details] [diff] [review]
basic implementation

updated to trunk
Comment 35 Justin Dolske [:Dolske] 2009-07-22 00:03:06 PDT
Comment on attachment 387833 [details] [diff] [review]
basic implementation

Generally looks ok, but I think there are a couple of things that need to be decided before getting into the details / landing...

* Are the existing controls sufficient, or do we really need to implement specialized controls for full-screen?

* What's required for a decent experience when switching to FS? This should be an instant change, without stopping to rebuffering, losing state, or altering the original page's DOM. roc had a newsgroup post about "proposal: using arbitrary elements as the source for CSS backgrounds" -- this is the best mechanism I've heard of so far. Now it just needs implemented. :)

* How to activate FS mode as a button in the video controls? That runs as unprivileged code, which makes things tricky. Maybe XBL2 will save us? A context-menu-only mechanism would be a fine first phase; but would we be ok with shipping just that?
Comment 36 Dão Gottwald [:dao] 2009-07-22 00:17:55 PDT
(In reply to comment #35)
> * What's required for a decent experience when switching to FS? This should be
> an instant change, without stopping to rebuffering, losing state, or altering
> the original page's DOM. roc had a newsgroup post about "proposal: using
> arbitrary elements as the source for CSS backgrounds" -- this is the best
> mechanism I've heard of so far. Now it just needs implemented. :)

I think we'd have to give up controls with that approach. Which brings us back to a suggestion from comment 9: "Any key event or mouse-down event could close the window."

> A context-menu-only mechanism would be a fine first phase; but would we be ok
> with shipping just that?

I don't see why not. Certainly better than no FS at all.
Comment 37 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-07-22 01:58:36 PDT
A big question is whether the "full screen" display should be tied to the underlying page's video element state, or forked into something with its own state. For example, if you start playing full-screen and page script decides to pause the video, should the full-screen display pause or not? If you go full-screen and pause manually, should the video element in the page appear to be paused?
Comment 38 Dão Gottwald [:dao] 2009-07-22 02:30:03 PDT
(In reply to comment #37)
> A big question is whether the "full screen" display should be tied to the
> underlying page's video element state, or forked into something with its own
> state. For example, if you start playing full-screen and page script decides to
> pause the video, should the full-screen display pause or not?

If the video is shared and the content page pauses or stops it, the full-screen window should probably close. That would be ok, I think, assuming that the page doesn't do it out of the blue. I don't really see another option other than not sharing the video.

> If you go full-screen and pause manually, should the video element in the page
> appear to be paused?

I'm not sure that proxy controls would be a wise idea if the video is shared. Maybe only if the page enables UA controls...
Comment 39 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-07-22 02:52:46 PDT
One approach, then, would be to share the video element state but redirect the rendering to your full-screen presentation. You can provide controls there that manipulate the original video element. If you see certain kinds of state changes (e.g., pause) that were not initiated from the full-screen controls, then you cancel the full-screen presentation.
Comment 40 John Drinkwater (:beta) 2009-07-23 16:22:28 PDT
(In reply to comment #37)
> A big question is whether the "full screen" display should be tied to the
> underlying page's video element state, or forked into something with its own
> state. For example, if you start playing full-screen and page script decides to
> pause the video, should the full-screen display pause or not? If you go
> full-screen and pause manually, should the video element in the page appear to
> be paused?

If the user left full-screen, assumption that the inpage video represents the position they were just at is high - its what most video players do now. So I imagine their states should be shared.

(In reply to comment #39)
> If you see certain kinds of state changes (e.g., pause) that were not initiated
> from the full-screen controls, then you cancel the full-screen presentation.

Though personally, once in full screen, the underlying page shouldn’t be able to manipute the video like it would if they did share state - if you’ve paused it and wandered off, wouldnt want it to be able to start again without user interaction or leave full screen.

I can see people using javascript playlists triggered at the end of the video in the future, so accepting some state changes are probably a must (but only when play is completed?)

How would javascript-controlled subtitles be affected by this?
Comment 41 :Ehsan Akhgari (busy, don't ask for review please) 2009-07-25 02:30:05 PDT
One of the things which we have advertised with <video> is the ability of web developers to replace our UI with their own JS UI.  I think we need to take this into consideration when thinking about fullscreen <video>.  If we simply open a new page and embed the video inside it and make it fullscreen, then we are practically ditching user's controls (and other things possibly, such as subtitles as mentioned in comment 40.)

Imagine a day when youtube decides to adopt <video>.  Their player currently provides a few buttons which are not available in our controls, such as the fullscreen button, the CC button, the HD button, etc (let's forget about the fact that we won't allow content to switch to fullscreen mode for a moment).  They are likely to provide their own JS controls imitating their current UI.  But when you go fullscreen, all of that would revert back to our own controls.  This means for example that users can't switch to the HD version or turn on CC in fullscreen mode in the future youtube, and also get a totally different player in that mode than what they get when they watch the video inline.

I'm not sure what a correct solution to this problem would be, though.  But I think this is a real problem which we should consider before implementing fullscreen support.
Comment 42 Dão Gottwald [:dao] 2009-07-25 02:42:05 PDT
There's no sane solution to that without new hooks for web content, which isn't within the scope of this bug.

I also don't think it's a real problem, but something that would be nice to have. "Don't let the perfect be the enemy of the good."
Comment 43 Biju 2009-08-01 19:16:21 PDT
(In reply to comment #41)
I feel it should be other way, when user go FullScreenVideo web page should not have any access to video. It should have only features what browser provides.
ie, just like what happens when user right click and do "View Video" or "View Image". I agree we will miss the possibility of CC or other similar features.

At present in Firefox 3.5 a website can provide FullScreenVideo with other features like CC.

what they need to do is just check for window.fullScreen and ask user whether he/she want "video full screen" or "web page full screen". If user want video full screen just size it to 100% of width and height.
Comment 44 orion.mozilla 2009-08-06 18:25:29 PDT
(In reply to comment #28)
> (In reply to comment #21)
> > Can somebody give this a try on Mac?
> > http://en.design-noir.de/mozilla/fullscreen-video/0.5.xpi
> 
> I started some work a week or so ago on a similar idea:
> 
> https://people.mozilla.com/~dolske/tmp/fullscreen-0.1.xpi
> 
> I used a <panel> instead of just a window, to work around OS X's titlebar
> problem. And tied it into the video controls (ugly hack!). I didn't pursue it
> much farther, though, because the performance on OS X was absolutely terrible.
> On my 1900x1200 monitor + new MacBook Pro, a full screen video would only play
> at ~3 fps and was basically unwatchable. At roc's suggestion I tried using
> CSS's "image-rendering: -moz-crisp-edges", but that didn't seem to help much.
> 
> Looks like there may be some perf issues with <panel>, because a fullscreen
> video with your extension seems to have a slightly higher frame rate. But it's
> still really bad. :-( I'd be curious to know how things fare on Windows and
> Linux.
> 
> My assumption for this bug is that we'd want a "real" fullscreen mode, meaning
> hardware scaling (as most FS video players do).

Justin, are we talking about the same extension?  Your creation has given me a much better experience than Flash ever did on Firefox 3+ on my GPU-less MacBook running OS 10.4.11.
Comment 45 Mike Beltzner [:beltzner, not reading bugmail] 2009-09-14 14:18:22 PDT
Boriss, you've nominated this for blocking, but I think it would need to make it for a beta (which should be in 1-2 weeks' time) and I'm not sure that we're that close to a patch here or to a set of solutions.

Could you round up the issues raised in previous comments and provide UX recommendations for Firefox 3.6, and file follow-up bugs for the issues you think we can tackle in Firefox 3.7 or a later release?
Comment 46 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-09-15 03:14:48 PDT
The patches in bug 513144 are up for review. That should fix the blocking backend issues, we'll be able to efficiently clone the video data from the original video into the fullscreen element and we won't chew up additional bandwidth downloading the same stream twice.
Comment 47 Dão Gottwald [:dao] 2009-09-21 04:31:39 PDT
Created attachment 401817 [details] [diff] [review]
patch

this uses mozLoadFrom and also adds a close button
Comment 48 Dão Gottwald [:dao] 2009-09-21 04:35:14 PDT
Note that this adds new localized strings for the context menu item. For 3.6, we could probably reuse fullScreenCmd.label.
Comment 49 Christopher Robert Jaquez 2009-09-21 05:37:48 PDT
Is it possible for this feature to interact with the OS to disable the screen saver during playback?  This is one thing that native players like Windows Media Player or VLC have over any in-browser (open video or flash-based) implementation that I have seen.  I assume flash cannot do this as a plug-in but the browser should be able to for fullscreen playback.
Comment 50 Dão Gottwald [:dao] 2009-09-21 05:39:47 PDT
I don't know. Could be investigated in a separate bug.
Comment 51 Dão Gottwald [:dao] 2009-09-21 12:21:43 PDT
https://build.mozilla.org/tryserver-builds/dgottwald@mozilla.com-try-1f0669cac5a2/

Linux didn't make it.
Comment 52 Biju 2009-09-21 19:41:34 PDT
are we going to get a fullscreen button also?
Comment 53 Biju 2009-09-21 20:28:36 PDT
+window.addEventListener("unload", function () {
+  contentVideo.currentTime = video.currentTime;

Will that make the spin wheel running when user returns from full screen

Dont we need some thing like
contentVideo.mozLoadFrom(video);
when user returns?
Comment 54 Dão Gottwald [:dao] 2009-09-22 00:58:08 PDT
(In reply to comment #52)
> are we going to get a fullscreen button also?

Not in this bug, I think.

(In reply to comment #53)
> +window.addEventListener("unload", function () {
> +  contentVideo.currentTime = video.currentTime;
> 
> Will that make the spin wheel running when user returns from full screen
> 
> Dont we need some thing like
> contentVideo.mozLoadFrom(video);
> when user returns?

The data is already shared between the two, I don't think another mozLoadFrom would make a difference.
Comment 55 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-09-22 01:51:54 PDT
Indeed, once you've done video.mozLoadFrom(contentVideo) the elements remain linked and any data loaded by one is available to the other.

Using contentVideo.mozLoadFrom(video) would actually be bad, since it would force contentVideo to go through the whole video loading lifecycle again (firing metadataloaded, loadeddata, etc).
Comment 56 Justin Dolske [:Dolske] 2009-09-22 20:34:37 PDT
Comment on attachment 401817 [details] [diff] [review]
patch

>+++ b/browser/base/content/fullscreen-video.xhtml

>+body.loadingdata > video,
>+body.loadingdata > #close,
>+body.idle > #close {
>+  visibility: hidden;
>+}

.idleUser (.userIdle, or whatever:), would be more descriptive as to what's idle.

Also, should the close button be on the left for OS X?

>+var contentVideo = window.arguments[0];

We're running as chrome here, right? Should be ok, though.

>+  video.mozLoadFrom(contentVideo);

Move this to after all the event listeners are hooked up? I don't think it should be a problem, but that seems better to do.

>+  video.addEventListener("loadeddata", function () {
>+    video.removeEventListener("loadeddata", arguments.callee, false);
>+    video.currentTime = contentVideo.currentTime;
>+    video.controls = true;
>+    video.play();
>+  }, false);

I think you'll also want to copy contentVideo.volume and .muted. And .poster might be useful too [if .play() is stalling without loading data, I think we'll still show the poster image].

Check with roc as to if a video without |autobuffer| can ever *not* fire loadeddata? If so, you might need to make this an earlier even (loadedmetadata), or set |autobuffer| on the <video>.

>+  // Copying the data from the content video may take a second or two. We hide
>+  // the video during that period.

I sort of wonder if we should have a throbber shown if it's taking too long. But it seemed fast to me, so we can do it later if it's a problem.

>+  video.addEventListener("seeked", function () {
>+    video.removeEventListener("seeked", arguments.callee, false);
>+    document.body.classList.remove("loadingdata");
>+    video.focus();
>+  }, false);

Why not just add this to the loadeddata listener? I'm actually mildly surprised this is working when the contentVideo's .currentTime is 0 (since that would == the fullScreen's .currentTime, and so I wouldn't expect a seeking/seeked to fire).

>+  // Automatically close this window when the playback ended, unless the user
>+  // interacted with it.
>+  video.addEventListener("ended", autoClose, false);

I think there's a slight bug here somewhere... If you drag the content video's scrubber to the end, and then fullscreen it, the full screen video pops up and then just closes a second later. It should just start playing from the beginning. [This was the only quirky thing I noticed while playing with the patch, though!]

>+  if (!video.controls &&
>+      String.fromCharCode(event.charCode) == " ")
>+    video.pause();
>+}, false);

Hmm. This should ideally handle all the key events that the controls normally would (eg, to adjust volume), but that would obviously be a bit of a mess. This is fine for now (the user just needs to wiggle the mouse first), but we should probably file a followup bug. This affects A11Y too. Maybe we can have the controls detect if they're privledged/full-screen, and either do the idle detection there (which would give us the gradual fade in/out for free), or do something so the key listeners stay active even when the controls are hidden...


>--- a/browser/base/content/nsContextMenu.js
>-    this.showItem("context-media-showcontrols", onMedia && !this.target.controls)
>-    this.showItem("context-media-hidecontrols", onMedia && this.target.controls)
>+    this.showItem("context-media-showcontrols", onMedia && !this.target.controls);
>+    this.showItem("context-media-hidecontrols", onMedia && this.target.controls);

*sigh*


>+  fullScreenVideo: function () {
>+    this.target.pause();
>+
>+    openDialog("chrome://browser/content/fullscreen-video.xhtml",
>+               "", "chrome,dialog=no", this.target);
>+  },

You could make mediaCommand() handle this, but it's ok this way too.


r+ with these changes...
Comment 57 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-09-22 20:37:12 PDT
> Check with roc as to if a video without |autobuffer| can ever *not* fire
> loadeddata?

A video should always fire loadeddata, even if not autobuffered (unless there's some kind of error).
Comment 58 Dão Gottwald [:dao] 2009-09-23 01:36:09 PDT
Created attachment 402305 [details] [diff] [review]
review comments addressed

> >+body.loadingdata > video,
> >+body.loadingdata > #close,
> >+body.idle > #close {
> >+  visibility: hidden;
> >+}
> 
> .idleUser (.userIdle, or whatever:), would be more descriptive as to what's
> idle.

changed to userIdle

> Also, should the close button be on the left for OS X?

yep

> >+  video.mozLoadFrom(contentVideo);
> 
> Move this to after all the event listeners are hooked up? I don't think it
> should be a problem, but that seems better to do.

done

> I think you'll also want to copy contentVideo.volume and .muted. And .poster
> might be useful too [if .play() is stalling without loading data, I think we'll
> still show the poster image].

done

> >+  video.addEventListener("seeked", function () {
> >+    video.removeEventListener("seeked", arguments.callee, false);
> >+    document.body.classList.remove("loadingdata");
> >+    video.focus();
> >+  }, false);
> 
> Why not just add this to the loadeddata listener?

Because seeking still takes a bit, and I think we don't want to show the throbber initially. I had to modify this, though, as it won't always seek.

> >+  // Automatically close this window when the playback ended, unless the user
> >+  // interacted with it.
> >+  video.addEventListener("ended", autoClose, false);
> 
> I think there's a slight bug here somewhere... If you drag the content video's
> scrubber to the end, and then fullscreen it, the full screen video pops up and
> then just closes a second later. It should just start playing from the
> beginning.

fixed
Comment 60 Asa Dotzler [:asa] 2009-09-23 09:27:35 PDT
I've played with this latest try server build for a while and it seems to all be working quite well on Mac. The only really uncomfortable part is interacting with the controls in full screen mode. They don't stick around when you've finished an interaction. For example, if I drag the scrubber, the second I let go the controls disappear and so I'm left not quite sure if it landed where I intended. Other than that, I think this is a great addition.
Comment 61 Mike Beltzner [:beltzner, not reading bugmail] 2009-09-23 10:05:05 PDT
Want this for beta; I'd encourage us to land initial support for b1 and file follow-up bugs for things like comment 60.
Comment 62 Sylvain Pasche 2009-09-23 10:20:16 PDT
Looking great on Windows too, some possible follow-up bugs I have in mind:
* Double clicking on the video should leave full screen mode, that's usually how full screen video players behave.
* Single clicking on the video should pause it?
* Have a lightweight version of the context menu in full screen with entries: Play/Pause, Mute, Leave Full Screen.
* In multi-monitor environment, the full screen window always opens on the main monitor, even when the Firefox window is located on another monitor when entering full screen.
Comment 63 Robert Kaiser (not working on stability any more) 2009-09-23 10:45:54 PDT
(In reply to comment #62)
> * Single clicking on the video should pause it?

Space key should pause/play in any case, btw, I guess those two can be connected in a single followup.
Comment 64 Justin Dolske [:Dolske] 2009-09-29 11:40:33 PDT
Bug 518008 already covers clicking on the video. I also mentioned here in comment 56 that we should probably make videocontrols.xml know about fullscreen more, so it can just handle the keyboard stuff there. Worth a followup...
Comment 65 Mike Beltzner [:beltzner, not reading bugmail] 2009-09-30 07:47:15 PDT
Boriss: can you do a ui-review here? The tryserver builds linked to in comment 59 should give you something to test with ...
Comment 66 Jennifer Morrow [:Boriss] (UX) 2009-09-30 18:38:10 PDT
Comment on attachment 402305 [details] [diff] [review]
review comments addressed

+r.  Great work on this, dao!
Comment 67 Dão Gottwald [:dao] 2009-10-01 01:51:18 PDT
http://hg.mozilla.org/mozilla-central/rev/51710e0ba8a3
Comment 69 David Bolter [:davidb] 2009-10-05 12:07:14 PDT
Apparently we assign an access key for this.
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-context.inc#106

.. but on a fresh mozilla-central trunk, on OS X, when I put focus on the video and try command-shift-f (which is the current access key I think) it just puts FF into full screen browse mode.

We should either make the access key work, or change the documentation to describe how to invoke the context menu (ctrl + space on mac). Probably the latter is the way to go (due to security-fu issues with the former).
Comment 70 Dão Gottwald [:dao] 2009-10-05 12:16:41 PDT
The access key is used in the context menu.
Comment 71 David Bolter [:davidb] 2009-10-05 12:22:24 PDT
Ah, okay that makes sense now thanks!
Comment 72 Tony Chung [:tchung] 2009-10-05 16:51:59 PDT
awesome work dao.  here's a couple observations i noticed in FS mode.  reply back if any of these are bugs are by design.  If they are bugs, i'll file separately.

Tested on video: http://tinyvid.tv/show/3gidxmf42584u, build: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091005 Namoroka/3.6b1pre

- when video plays, right-click context menu is disabled
- on a mac, middle clicking the mouse brings the mac widgets overlaying over the FS video.   With a flash video in FS, middle-click immediately brings video out of FS.
- when the control bar is hidden, arrow keys seeking is disabled.  
- with a secondary monitor (desktop sharing mode), drag your browser to the 2nd screen, then go to FS.  you'll notice the FS occurs on the first monitor, while the browser video is paused.   In flash, FS applies to the screen that has focus.
Comment 73 orion.mozilla 2009-10-05 17:06:05 PDT
(In reply to comment #72)
> - on a mac, middle clicking the mouse brings the mac widgets overlaying over
> the FS video.   With a flash video in FS, middle-click immediately brings video
> out of FS.

From the sound of it, I think I'd prefer that behavior to Flash's.  Then again, I'm not a big widget user anyway.
Comment 74 Dão Gottwald [:dao] 2009-10-06 05:48:07 PDT
(In reply to comment #72)
> - when video plays, right-click context menu is disabled

There's no context menu at all in full-screen playback mode. I don't think we need one, but maybe I'm wrong.

> - on a mac, middle clicking the mouse brings the mac widgets overlaying over
> the FS video.   With a flash video in FS, middle-click immediately brings video
> out of FS.

I don't know the purpose of these widgets, so I don't know if we wanted to fix this. What we probably should do is close the full-screen window whenever it loses focus, depending on bug 511503.

> - when the control bar is hidden, arrow keys seeking is disabled.

Not really wanted, but a known quirk. We'll need a bug for that.

> - with a secondary monitor (desktop sharing mode), drag your browser to the 2nd
> screen, then go to FS.  you'll notice the FS occurs on the first monitor, while
> the browser video is paused.   In flash, FS applies to the screen that has
> focus.

Yeah, we'll also need a bug for that.
Comment 75 u88484 2009-10-06 06:07:46 PDT
(In reply to comment #74)
> (In reply to comment #72)
> > - when video plays, right-click context menu is disabled
> There's no context menu at all in full-screen playback mode. I don't think we
> need one, but maybe I'm wrong.

I filed a bug on this a few days ago, bug 520160
Comment 76 Tony Chung [:tchung] 2009-10-06 10:16:52 PDT
(In reply to comment #74)
> > - when the control bar is hidden, arrow keys seeking is disabled.
> 
> Not really wanted, but a known quirk. We'll need a bug for that.

Filed bug 520806
> 
> > - with a secondary monitor (desktop sharing mode), drag your browser to the 2nd
> > screen, then go to FS.  you'll notice the FS occurs on the first monitor, while
> > the browser video is paused.   In flash, FS applies to the screen that has
> > focus.
> 
> Yeah, we'll also need a bug for that.

Filed bug 520805

Also marking this bug verified on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091006 Namoroka/3.6b1pre.   Fullscreen video has been implemented.
Comment 77 Tony Chung [:tchung] 2009-10-06 11:37:26 PDT
Added a litmus FFT testcase for FS.  https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7060

Note You need to log in before you can comment on or make changes to this bug.