The default bug view has changed. See this FAQ.

Support for fullscreen video playback

VERIFIED FIXED in Firefox 3.7a1

Status

()

Firefox
General
P1
enhancement
VERIFIED FIXED
9 years ago
2 years ago

People

(Reporter: Dolske, Assigned: dao)

Tracking

(Depends on: 1 bug, {verified1.9.2})

Trunk
Firefox 3.7a1
verified1.9.2
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3.6 +
in-litmus +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

9 years ago
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.
Flags: wanted1.9.1?
Not for 3.1 I think. Maybe the following release.
Flags: wanted1.9.1? → wanted1.9.1-

Comment 2

9 years ago
Hooray, another kick in the **** for open standards. Why oh why do I bother. Adobe Flash, here I come!
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

9 years ago
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

9 years ago
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

8 years ago
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

8 years ago
(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

Updated

8 years ago
Blocks: 470628
(Reporter)

Updated

8 years ago
Duplicate of this bug: 470628
No longer blocks: 470628
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...
(Reporter)

Comment 10

8 years ago
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

8 years ago
> 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.
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.
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.

Updated

8 years ago
Duplicate of this bug: 499428

Comment 15

8 years ago
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

8 years ago
Marking as wanted for 1.9.2 as that is what seems to be possible.
Flags: wanted1.9.2?
Flags: wanted1.9.2?
Flags: wanted1.9.2+
Flags: blocking1.9.2?

Comment 17

8 years ago
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

8 years ago
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

8 years ago
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

8 years ago
(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.
(Assignee)

Comment 21

8 years ago
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.
Depends on: 370857

Comment 22

8 years ago
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

8 years ago
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.
(Assignee)

Comment 24

8 years ago
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.
(Assignee)

Comment 25

8 years ago
Created attachment 386769 [details] [diff] [review]
basic implementation
Attachment #386769 - Flags: review?(dolske)
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

8 years ago
I tried the extension from comment 21 on Linux. It works very well!
(Assignee)

Updated

8 years ago
Depends on: 502440
(Reporter)

Comment 28

8 years ago
(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).
(Assignee)

Comment 29

8 years ago
(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.
(Assignee)

Updated

8 years ago
Assignee: nobody → dao
Status: NEW → ASSIGNED

Comment 30

8 years ago
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.
(Assignee)

Comment 31

8 years ago
(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

8 years ago
(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

8 years ago
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.
(Assignee)

Comment 34

8 years ago
Created attachment 387833 [details] [diff] [review]
basic implementation

updated to trunk
Attachment #386769 - Attachment is obsolete: true
Attachment #387833 - Flags: review?(dolske)
Attachment #386769 - Flags: review?(dolske)
(Assignee)

Updated

8 years ago
Component: Video/Audio → General
Flags: wanted1.9.2+
Flags: wanted1.9.1-
Flags: blocking1.9.2?
Product: Core → Firefox
QA Contact: video.audio → general
(Assignee)

Updated

8 years ago
Flags: blocking-firefox3.6?

Updated

8 years ago
Blocks: 467530
(Assignee)

Updated

8 years ago
Depends on: 476371
(Reporter)

Comment 35

8 years ago
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?
Attachment #387833 - Flags: review?(dolske)
(Assignee)

Comment 36

8 years ago
(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.
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?
(Assignee)

Comment 38

8 years ago
(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...
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.
(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?
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.
(Assignee)

Comment 42

8 years ago
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."
(Assignee)

Updated

8 years ago
Depends on: 506826

Comment 43

8 years ago
(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

8 years ago
(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.
(Assignee)

Updated

8 years ago
Flags: blocking-firefox3.6?
(Assignee)

Updated

8 years ago
Depends on: 513144
Flags: blocking-firefox3.6?
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?
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.
(Assignee)

Comment 47

8 years ago
Created attachment 401817 [details] [diff] [review]
patch

this uses mozLoadFrom and also adds a close button
Attachment #387833 - Attachment is obsolete: true
Attachment #401817 - Flags: ui-review?(jboriss)
Attachment #401817 - Flags: review?(dolske)
(Assignee)

Comment 48

8 years ago
Note that this adds new localized strings for the context menu item. For 3.6, we could probably reuse fullScreenCmd.label.
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.
(Assignee)

Comment 50

8 years ago
I don't know. Could be investigated in a separate bug.
Blocks: 517870
(Assignee)

Comment 51

8 years ago
https://build.mozilla.org/tryserver-builds/dgottwald@mozilla.com-try-1f0669cac5a2/

Linux didn't make it.

Comment 52

8 years ago
are we going to get a fullscreen button also?

Comment 53

8 years ago
+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?
(Assignee)

Comment 54

8 years ago
(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.
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).
(Reporter)

Comment 56

8 years ago
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...
Attachment #401817 - Flags: review?(dolske) → review+
> 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).
(Assignee)

Comment 58

8 years ago
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
Attachment #401817 - Attachment is obsolete: true
Attachment #402305 - Flags: ui-review?(jboriss)
Attachment #401817 - Flags: ui-review?(jboriss)
(Assignee)

Updated

8 years ago
No longer depends on: 476371
(Assignee)

Updated

8 years ago
No longer depends on: 506826
(Assignee)

Updated

8 years ago
No longer depends on: 502440
(Assignee)

Comment 59

8 years ago
https://build.mozilla.org/tryserver-builds/dgottwald@mozilla.com-try-daa170040ddf/

Comment 60

8 years ago
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.
Want this for beta; I'd encourage us to land initial support for b1 and file follow-up bugs for things like comment 60.
Flags: blocking-firefox3.6? → blocking-firefox3.6+
Priority: -- → P1

Comment 62

8 years ago
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.
Flags: blocking-firefox3.6+ → blocking-firefox3.6?
Priority: P1 → --
Flags: blocking-firefox3.6? → blocking-firefox3.6+

Updated

8 years ago
Priority: -- → P1

Comment 63

8 years ago
(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.
(Reporter)

Comment 64

8 years ago
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...
Boriss: can you do a ui-review here? The tryserver builds linked to in comment 59 should give you something to test with ...
Attachment #402305 - Flags: ui-review?(jboriss) → ui-review+
Comment on attachment 402305 [details] [diff] [review]
review comments addressed

+r.  Great work on this, dao!
(Assignee)

Comment 67

8 years ago
http://hg.mozilla.org/mozilla-central/rev/51710e0ba8a3
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
(Assignee)

Comment 68

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/98c2b0def6b4
status1.9.2: --- → beta1-fixed
Depends on: 520600
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).
(Assignee)

Comment 70

8 years ago
The access key is used in the context menu.
Ah, okay that makes sense now thanks!
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

8 years ago
(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.
(Assignee)

Comment 74

8 years ago
(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

8 years ago
(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
(Assignee)

Updated

8 years ago
Depends on: 520160

Updated

8 years ago
Blocks: 520805

Updated

8 years ago
Blocks: 520806
(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.
Status: RESOLVED → VERIFIED
Keywords: verified1.9.2

Updated

8 years ago
Blocks: 520811
Added a litmus FFT testcase for FS.  https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7060
Flags: in-litmus+

Updated

8 years ago
Blocks: 521129
(Assignee)

Updated

8 years ago
No longer blocks: 517870, 520805, 520806, 520811, 521129
Depends on: 517870, 520805, 520806, 521129

Updated

7 years ago
No longer blocks: 467530
Blocks: 531576

Updated

4 years ago
Depends on: 811261

Updated

4 years ago
Depends on: 830660

Updated

4 years ago
Depends on: 844444
(Assignee)

Updated

4 years ago
No longer depends on: 844444

Updated

2 years ago
Depends on: 968526

Updated

2 years ago
Depends on: 1064787
You need to log in before you can comment on or make changes to this bug.