Closed Bug 448909 Opened 16 years ago Closed 16 years ago

Need more controls WHATWG Video tag

Categories

(Toolkit :: Video/Audio Controls, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: BijuMailList, Assigned: Dolske)

References

Details

(Keywords: verified1.9.1)

Attachments

(3 files, 10 obsolete files)

Bug 422538, implemented support for <VIDEO>
But when playing 
http://commons.wikimedia.org/wiki/Image:Fraser_island_australia_sand_track.ogg 

I cant do replay at the end.

We should have controls for
* Play (exist now)
* Pause (exist now)
* Re-play 
* Slider for positioning frame to play
* Slider for volume control
* button for sound on/off
* Theater Mode (Full Screen)
* keyboard: space bar for Play/Pause
* keyboard: Esc for exit Theater Mode
* mouse: click for Play/Pause
* mouse: dblclick for Theater Mode
Summary: Implement WHATWG Video spec → Need more controls WHATWG Video tag
Attached file video_iframe.html (obsolete) —
We are already allow to over follow when placing control on small sized video.

But what if somebody put a small IFRAME and then page with VIDEO tag in that like video_iframe.html (also need another test case with an IFRAME then FRAMESET then page with VIDEO)

We need the controls to appear on the top most window/frame.

Also consider providing controls on Status bar and Toolbar.
May be that is another bug for UI
http://www.whatwg.org/specs/web-apps/current-work/#controls expects user agents to provide seeking, volume, and full-screen controls.

I'd love to see more powerful seeking controls than YouTube provides.  Having only a seek bar gets old fast when trying to move around within a long video.
* Left arrow key to go back ~5 seconds.
* Right arrow key to go forward ~5 seconds.
* Click the time (e.g. 2:29 / 2:35) to enter a time to seek to (e.g. 1:40).  Bonus points if the interface is inline rather than modal.
* Ability to move back or forward one frame at a time while paused.

I like the way YouTube keeps the volume controls small.  It seems they understand that most users will use volume controls on their keyboards or speakers rather than the ones associated with the video.  HTML5 requires the default video volume to be the loudest, so we should do the same.

What is the Stop button for?  As far as I can tell, it does the same thing as Pause.
I worry "double-click to enter theater mode" or "click this button enter theater mode" could be a security hole due to the potential for chrome/OS spoofing.  We should at least implement some basic security measures:

1. Make sure web pages don't receive key or mouse events while the video is full-screen.

2. Show a "Press Esc to exit full-screen mode" overlay for a few seconds.  This imposes additional constraints on an attack, even if users don't manage to read the overlay itself, as in this amusing Flash demo:
http://www.bunnyhero.org/2008/05/10/scaring-people-with-fullscreen/.

3. Show video controls temporarily if the mouse cursor is moved or an irrelevant key is pressed.

Even then, I am concerned that a web page might use the spoofing opportunity to prime a victim for later information disclosure or compromise.  Maybe we should make users click twice to get into full-screen mode: once on a button that opens a menu, and then on a "full screen" option in the menu.  The menu could also contain a "move video to a separate window" command.

I'm probably just being over-paranoid.  If everyone else thinks a full-screen button is safe enough, I won't veto the button.
We should definitely have some UX folks weigh in on what the <video> control UI should look like (assuming this is the right bug for it).
UX folks should feel free to take ownership of this. The controls are just an XBL binding that use the public <video> element interfaces, and they're in toolkit, so go wild.

BTW we also need controls for <audio>, right now we don't have any. Although it's not clear how big an <audio> element with controls should be.
One option for full-screen mode is to simply allow the video controls to take focus, and then if the user presses F11 while a video element is focused, make the video full-screen instead of just the page. Then make the key discoverable with a tooltip or something.
I don't think we should sacrifice the usability and immersiveness of the video experience on the alter of paranoia, but I think things like Jesse's #2 & #3 should help cue users without being overly intrusive.

I haven't thought hard about this yet, but some way of making users comfortable with the mechanisms they have for breaking out of the fullscreen environment feels like an important part of keeping them in control of their browser.
Component: DOM: Core & HTML → Video/Audio
QA Contact: general → video.audio
I'd settle for just:

* Play (exist now)
* Pause (exist now)
* Re-play 
* Slider for positioning frame to play
* Slider for volume control
* button for sound on/off

As a good starter.  Who on the UX team can help with the visual design of this?  Would be great to be able to demo something with b1 that shows a playing experience closer to that of youtube or other sites that folks are familiar with.  In particular not being able to restart and/or seek is pretty problematic.

if this is harder to do as native controls I'd be thrilled with a YUI slider skin (http://developer.yahoo.com/yui/slider/) that looked more like  play/pause + seek + mute controller.

Sounds like seeking will be blocked by bug 449159 right now.      

As a second pass being able to show how much of the video has downloaded would be super-helpful.  Need bug 449307 for that to work.
Depends on: 449282, 449307
Assuming that UX is to provide the visuals for the first pass at this, we'll need to know what sort of resources you guys need. Don't presume we're XBL experts in terms knowing what you'd want to provide.

Are we talking a set of images (if so, what formats?)
Do we have someone who can put together the CSS?

It feels like we're coming at this cross-purposes, which is to say that there are actually two requests:

1. What are the set of controls and control mappings we want to provide by default with the <video> element?

2. What visuals should we provide by default? Should those visuals be themable?

My gut feeling is that we should do like we've always done, and take guidance on visuals from the OS theme in question.
Should have added: is there a document that describes what sort of level of control we have over the object? While I agree that we should iterate towards success here, it would be nice to know what is within the bounds and what isn't, if only so that we can file bugs to have them put into those bounds ;)
Adding Wei who will deliver a few mockups of how the default controls might look.
The WHATWG spec outlines the public interface:

http://www.whatwg.org/specs/web-apps/current-work/#video
http://www.whatwg.org/specs/web-apps/current-work/#media

This is the interface that the existing videocontrols.xul uses.
I could do it. How urgent it is? What's the deadline? 
According to comment #8 we want this for beta1, which means code checked in by August 19, which means we need to be working on it now. With any luck Chris will have bugs 449282 and 449307 fixed by then. The UI can proceed in parallel since HTML5 has already defined the API for us.
Can the controls overlay the video?
I did a fast design here:  

http://www.weizhoudesign.com/controls.jpg

But I am still confused what format is needed? A png contains all the UI
elements? or separate single UI element?

(In reply to comment #18)
> I did a fast design here:  
> 
> http://www.weizhoudesign.com/controls.jpg
> 
> But I am still confused what format is needed? A png contains all the UI
> elements? or separate single UI element?
> 

This will be done in XUL - so separate elements as html/css/images would be ideal methinks.
i'm pretty sure the ideal deliverable is something fairly similar to xul or html (basically the xbl will embed xul [or in worst case some xhtml]).

xul offers a <progress> control which will pick up os theming, although it's not necessarily the equivalent of a track control, and i don't think we have theming support for the track element (which is actually a fairly complicated widget).

i'm also fairly certain we don't have any real vertical sliders other than scrollbars.

if you're going to offer time (and i'm assuming that's expected) you should define how to handle a 3 day long movie in addition to a 2s movie. do we ever show microseconds? do we show both total time and time elapsed / time remaining? (please check the ipod video player).

how is muting handled?

if you're going to define your own track widgets, then you really need to create your own xbl widgets for them and work w/ the os themers to let them replace your widgets w/ native themable elements. This is a lot of work (and requires coordination across 3 platforms). I'd recommend using a progress meter for now and try to theme later, although if you want to do theming, you probably need to give the os widget people a heads up *now*.
(In reply to comment #20)
I think xul:scale is pretty much what we want.
comment 20 asks some good questions, and this is why I think there are separate exercises here: the first is to get something "better than what we have now" and the second is to figure out what controls we want to provide in the long term.

For the short term, I agree with Dao that a horizontal xul:scale for the scrub bar, a vertical xul:scale for the volume, and some toggle buttons for play, pause and mute are a minimum bar for beta.

Wei's provided some bare-bones graphics; I believe that those can be chopped up into the PNGs required by the xul:scale CSS, and other CSS can be made to match. I'd rather see an overlay design (as Alex suggests) though since that means we don't need to add anything beneath the <video> element. 

Wei: can you do another pass at your mockup, with the same controls but on a light grey background that we would lay on top of the video when the mouse hovers over it?

For the long term, we will likely want to create a new widget for this that includes:

 - track bar which allows random access, chapter/scene marks (see TED Talk Player) & shows download progress
 - play/pause button
 - mute button
 - volume slider
 - time elapsed / remaining indicator (toggles on click?)
 - options button (save to disk, copy url, copy embed code, content-side, seek to time, configurable actions?)
Flags: blocking1.9.1?
Attached image example of overlay controls (obsolete) —
Here's a good example from globeandmail.com

 - when paused, overlay entire video with clear "click to play" icon
 - controls slide up when user hovers over
 - scrub bar always shown at bottom of video (small)

Not sure the scrub bar actually always needs to be there (might get annoying for longer videos) but I like the contrast & size they chose.
Depends on: 449539
(In reply to comment #22)
> exercises here: the first is to get something "better than what we have now"
> and the second is to figure out what controls we want to provide in the long
> term.

In long term we (and all other browsers) should provide something better than currently existing out there with Flash. With out an add-on value, I dont think VIDEO providers like YouTube have any temptation to use VIDEO tag as an alternative.
No longer depends on: 449539
I'll be working with Faaborg on this.
Assignee: nobody → dolske
Nothing exists, AIUI, "with Flash". There are several licensed and free web based FLV players available such as:

http://flv-player.net/players/mini/
http://www.flowplayer.org/

(ref: http://en.wikipedia.org/wiki/Flash_Video#Web-based )

The value add for web content authors is being able to control their video through HTML instead of having to communicate through the plugin layer.
(In reply to comment #24)
> In long term we (and all other browsers) should provide something better than
> currently existing out there with Flash. With out an add-on value, I dont think
> VIDEO providers like YouTube have any temptation to use VIDEO tag as an
> alternative.

(In reply to comment #26)
> The value add for web content authors is being able to control their video
> through HTML instead of having to communicate through the plugin layer.

Specifically, content providers can create their own controls, so even if we wouldn't provide a more compelling UI, that wouldn't be the end of <video>.
Attached file slider widget (obsolete) —
Attached file slider widget
yay, box-shadow!
Attachment #333603 - Attachment is obsolete: true
So far the controller looks like this:

http://www.weizhoudesign.com/player.png

Here're some feedbacks:
- pretty OSX centric, but it will probably look OK on Windows and we
can do more themeing later
  - when clicked, the volume slider should be attached to the volume overlay
  - on the bottom set of controls, I don't think the volume button needs
to be sitting on a tray; or is the idea that you can turn the volume on
and off?
  - I assume that last button is "full screen", the icon isn't very
clear; maybe use arrows pointing to the corners?
  - in the upper of the two layouts, I don't understand what the darker
area represents as it doesn't extend all the way to the left - is that
just a mistake? does it represent the available downloaded content?
  - are the dots bookmarks or jump points?
  - while I understand the desire to use the blue insertion marker for
consistency with the tabstrip, I don't think that it is consistent with
the rest of the video controls; I would expect a thicker line with some
indication that it can be grabbed and moved

* Why have the volume be activated as a click at all. You can do it in-line and save a two-part workflow. (See the vimeo player, or the Songza player)
* Play/pause toggles are known to cause mode problems, especially with laggy video. Does the two bars mean the video is currently paused, or that clicking it will cause it to pause. It's hard to tell if the video is stuttering due to bandwidth. We tried to solve this with the Songza player by having an explicit distinction between play and pause. (This could easily fit into the keyhole motif). Other methods are to add some words so that the current status of the video is always known.
* You might consider making the scrub-head marker extend to extend out of the top of the track.
* I like the idea of built-in chapters/interest marks. Should we think about extending that to easily include social aspects, like commenting, or further chaptering (like the TED conference video player).
* Thoughts on subtitles, switching languages, etc?
* Opposed to what Belzner said, I'm not sure that I'd make the scrub-head look like it is draggable. That will cause people to try to drag it instead of clicking on the track to scrub. The former interaction involves targeting a tiny target, clicking, and dragging, whereas the latter is an easier targeting problem and a single click. The second is much master, and we should encourage that behavior.
* Another option is to let the call-out be draggable (adding left/right arrows on it). Again, see Vimeo.
* Embedding options?
* Do we care about showing total video time?
http://www.weizhoudesign.com/player2.png

You could find the answers in the picture above, here are some extra thoughts:

1. I'm thinking about adding a "info" button somewhere, in that button, we may show additional options like"embed the video, share the video, show video info, show chapter info, show comments, subtitles, language,  etc."

2. Make the scrub-head marker extend(out) is cool. That's actually what I wanted to do at the beginning. But I ended up with a conservative design. I remember Alex gave me four criteria. Actually even for now, I still feel like the whole thing is over-designed - should it be more subtle?
* Consistent with Firefox browser's UI
* Consistent with the Operating System UI
* Conservative and plain UI style but not boring
* Loading Speed is key.

3. Aza thought the dots are interest marks - I thought of advertising, but I think interest marks is more fun. 

"3. Aza thought the dots are interest marks - I thought of advertising, but I
think interest marks is more fun."

What are these in relation to the video API described in the WHATWG specification? Are they the cue points?  
Media player UI update:

http://www.weizhoudesign.com/player3.png


Chris: YES, they are cue points. 
Questions

1. What's the UI size, for a 800*600 video,a 400*300 video, or a wide screen video? how do we deal with irregular size video?What if the video size is smaller than the UI?

2. How do we design the full screen player UI? Do we need to consider different screen resolutions?

3. Different styles for Windows, Mac or Unix?

4. If separate SVG UI files are needed, I still need to know how the code is written to "separate" them. For instance, In flash's UI I prefer to render a 1x10 pixel image 10 times rather than rendering a 10x10 one - for faster loading speed. 

Well, I'll end my internship this Friday. Hopefully I could get something done before that. Who is the person I'm directly responsible to? 


IRC: wei 

>1. What's the UI size, for a 800*600 video,a 400*300 video, or a wide screen
>video? how do we deal with irregular size video?What if the video size is
>smaller than the UI?

>2. How do we design the full screen player UI? Do we need to consider different
>screen resolutions?

Since we are using SVG for the UI, we should decide on some padding ratios so that the UI works with a variety of different sizes of video (so in full screen the controls would simply be larger).  How about we sit down with dolske tomorrow to discuss what to do in really extreme cases of video size.

>3. Different styles for Windows, Mac or Unix?

Let's do one generic style for now just to get things landed.  I'm a little worried that the metal appearance of the current design might cause users to incorrectly assume that <video> is somehow related to iTunes on windows.  Perhaps we should try a transparent black UI (with a white outline around the controls and white glyphs, in case the video it is overlaying is itself black).

>4. If separate SVG UI files are needed, I still need to know how the code is
>written to "separate" them.

Here is an example of svg controls with xul bindings:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml

From the example it looks like they are doing everything with svg (no raster graphics), and surrounding the svg for each control are xul buttons with particular classes and labels:

<xul:button class="centerplay" ... label="Play">

let's get what you have so far into a working implementation and then we can continue iterating on the visual and interactive design.

>Who is the person I'm directly responsible to? 

In terms of implementation work with Dolske to make sure he has everything that he needs.  Beltzner and myself will continue giving you feedback on the design.
Forgot to add: really great work on the first pass of the UI Wei!
is someone considering accessibility, or do we back engineer that later?
Attached file First pass at base structure (obsolete) —
This is a first pass at the the basic structure for jamming the pretty things into. The main concern here was getting all the various boxes to scale in a sensible way. [As it turns out, combining XUL+CSS+SVG is a bit headache inducing.] Overall seems to work well, the exception being the edge case of an extremely wide-but-short video (eg, 1000x20) -- there's extra dark-blue SVG area (to the right of the yellow button) that I'm not sure how to suppress. I think it's a side effect of SVG scaling to fit plus XUL distributing flex space.
Svg files could be downloaded here:

www.weizhoudesign.com/Player_SVGs.zip

Tomorrow I'll leave Mozilla, but work as a design contributor, be free to email me anytime.




Attached file First pass at base structure (obsolete) —
Did some more work from the last version, but I don't think this approach is going to work. Things causing problems:

* XUL boxes of unequal size that are flexed will scale in a mismatched way (here, the right end cap is intended to be twice as wide as the left end cap, but they scale at different rates). Not sure how to make XUL do what I want.

* SVG doesn't seem to be able to scale in a useful way for the progress bar bits, because it wants to preserve the aspect ratio of its viewbox. You can draw at relative positions within the viewbox ("draw a dot 33% from the left"), but you can't make the viewbox itself fill up an arbitrary space (eg, an area that might be 3x1 or 11x7). Well, there's preserveAspectRatio="none", but then anything drawn gets squished.

* When the endcaps are scaled down to fit in their views, there's space being wasted that would ideally be used to make the progress bar section wider.

I'm not sure if these problems are solvable with declarative markup, but I think it should be possible to set up the element positioning with JS. EG, instead of trying to cleverly glue pieces together with XUL+CSS+SVG, just calculate where the pieces should go and place them there directly. Repeat if the <video> is resized.
Attachment #334802 - Attachment is obsolete: true
(In reply to comment #39)
> Svg files could be downloaded here:
> 
> www.weizhoudesign.com/Player_SVGs.zip

As suspected, the SVG output from Adobe Illustrator is really bad to work with. It's big blobs of indecipherable <path>s. :-( But I think that since the controls are fairly simple, it won't be a big deal to hand-code some SVG to exactly match the appearance of your design.
Has any thought been given to a right click context menu?

In my opinion, it should include access to more advanced features like:
Open media in external player
Selection of video stream
Selection of Audio stream
Selection of captions
Video Properties
video size scaling (maybe)

It looks like javascript is going to be required to lay out the controls, because in the case of small or oddly sized videos you might want to lay out the UI elements differently (for example remove the less important buttons on tiny videos)


If I wanted to test an implementation of the video interface XUL, is there any way to do it without recompiling firefox or the theme?
(In reply to comment #42)
> Has any thought been given to a right click context menu?

see bug 449522
Maybe Neil, Jonathan or Robert can help with comment #40
Depends on: 452031
Attached file Base structure v.2 (obsolete) —
*sigh* More fail. This is sizing the controls from JS, but has a few serious problems:

* I think I'm hitting some bug in SVG. I'm getting strange sizes for the viewport, and things are not being drawn where I'd expect. Not sure what's happening, filed bug 452031.

* I can't seem to get XUL "resize" events to fire on an element. Google leads me to suspect this either never worked, or only worked on <window>. Not sure if this can be fudged somehow (timers? onmouseover? onoverflow? mutation events?). In the meantime, I've just added a button in the mockup to click when resizing the window.
Attachment #335234 - Attachment is obsolete: true
> Here is an example of svg controls with xul bindings:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml

Why was badly written and inaccessible XUL code checked in to toolkit without any module owner/peer review?
Or maybe you need a pixel version as well as a hand-written SVG file? 

Let me know what do you need for now. 

(In reply to comment #46)
> > Here is an example of svg controls with xul bindings:
> > http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/videocontrols.xml
> 
> Why was badly written and inaccessible XUL code checked in to toolkit without
> any module owner/peer review?

That was my fault, sorry. But it is going to go away.
Attached file Base structure v.3 (obsolete) —
Some progress; fixes the positioning, and the scrubber responds to mouse events. Next up is to move to XBL and start making things less hacky. I think I'll also need to not draw the controlbar background in segments, as the background can leak through the joints (here, the thin vertical lines left/right of the scrubber).

Still no idea how to redraw the controls when video's size changes. Maybe we can have the <video> code generate a VideoResized event and listen for it? This might be needed anyway, if we want the controls to be sized/positioned to the actual size of the rendered video, and not just the <video> box. [Eg, given <video id="vid" width="400" height="300" src="tvshow_4x3.ogg">, and a script later does |vid.src = "movie_widescreen.ogg"|.]
Attachment #335329 - Attachment is obsolete: true
Attached file XUL+SVG layout (obsolete) —
I think we should be able to avoid having to lay everything out by hand with JS. (If we can't, we should fix that in our platform pronto!)

Here's my attempt at doing things right with XUL hboxes, stacks, and SVG. Avoids seams and doesn't require any manual JS layout. Justin, is there anything missing that's not easy to build on top of this structure?
There currently is no way to position the controls inside the actual video area, if we're doing letterboxing or whatever. But I'm not even sure if that's a good idea. If we want to do that, we could have nsVideoFrame explicitly size and position the anonymous controls element in C++.
Blocks: video
(In reply to comment #51)
> There currently is no way to position the controls inside the actual video
> area, if we're doing letterboxing or whatever. But I'm not even sure if that's
> a good idea. If we want to do that, we could have nsVideoFrame explicitly size
> and position the anonymous controls element in C++.
If I understand that correctly, does that mean that we cannot have the controls over the video and that they have to be below it?
No, the question was about when the video content is scaled to fit within the <video> box. (eg, a widescreen video in a square <video> tag).
Having standard keys that work whether or not the visual controls are visible would be great.

Having a mute or pause key that always works is especially important because automatically loading video/audio interferes with screen readers. The user obviously can't just do a system-wide mute or they can't access their system at all.
for autoplaying content, since that will obviously interfere with screen readers, wouldn't the right thing be for the system to suppress autoplay and have the screen reader announce that there's media which would like to play and then have the screen reader control when it plays?

i understand aria has a way to notify users of content/changes, it seems like audio/video (but especially *hidden* audio) should include that.

that said, short of a sound manager, i think users of screen readers are going to be in trouble, as soon as they have a window that tries to run three related sound tracks (as distinct audio files) at the same time (background, accompaniment, vocals). trying to silence all of them will be painful.
Tineless, good thoughts.

I have a different but related issue. Sometimes if I accidentally click on the same link twice I get 2 tabs, and they both start speaking.

Or, one is taking too long and I'm loading another.

It's annoying to have 2 speaking at once. Maybe if we're implementing our own video and audio we can have a UI to deal with that. Not sure if it's exactly related to the screen reader interference problem or not.
Filed bug 455486 on the idea for enhancement of a media manager. Given how a lot of people surf many videos at once from different tabs, I think it would help.
Attached patch Patch v.1 (WIP) (obsolete) — Splinter Review
From discussions since my last update:

* We don't need to scale the video controls, they'll be a fixed size (other than the scrubber, which will stretch horizontally). This has two implications:
  - There's no need to use SVG here, we can just use standard XUL + images like the rest of the UI.
  - The controls will be too big for a tiny video (for some value of "tiny"). We should probably have a specialized set of controls for this case -- even if you scaled the controls, a scrubber or pause button that's just 2 or 3 pixels wide is useless. We might want, say, a single play/pause button that covers most of the <video>. This needs design work.

* Boriss has been doing some updated designs, we'll probably move away from the stylized controls Wei did to a more neutral set of controls (like most web video players have).

Attached is, again, a first pass at implementing some of this. This time as a real patch, and not a XUL mockup. :)
Attachment #332102 - Attachment is obsolete: true
Attachment #332543 - Attachment is obsolete: true
Attachment #335774 - Attachment is obsolete: true
Attachment #335819 - Attachment is obsolete: true
Depends on: 455921
No longer depends on: 452031
I should note that it will be hard to make more progress on this, functionally speaking, without some of the bugs blocking this one... The scrubber needs bug 449282 to be able to seek, and we can't show the current play position without bug 449307. I'm also wary about the complexity of all the events and states in the HTML5 spec... Having the UI handle all the combinations will probably take more works than it might seem at first glance.

Enn: mconnor suggested you would probably be the eventual reviewer for this stuff -- anything we should go over while I'm in town?
Those two bugs are stalled waiting for bug 449159 which hopefully will be done and landed real soon now. Once that's in I'll get 449282 and 449307 done fairly quickly.
> Enn: mconnor suggested you would probably be the eventual reviewer for this
> stuff -- anything we should go over while I'm in town?

Were there specific questions you wanted to ask? Or are you asking for a review now?

This version is much better than the other one from a cursory glance.
Dolske: can we get at least a first version (play, pause, mute) landed for beta 1 and file a follow up to specifically add scrubbar functionality? Probably better to start cutting this up into a bunch of little additions to some basic frame of functionality.
Attached patch Patch v.2 (obsolete) — Splinter Review
Cleaned up patch, implements just play/pause/mute. Unfortunately this is going to miss Beta 1 unless Enn or someone wants to do a last-minute review. :-(

This patch also removes the nsISecurityCheckedComponent stuff. Apparently the problem doublec ran into back in January (http://tinyurl.com/4cve5n) has somehow been fixed, and this isn't needed to make things work now.
Attachment #339295 - Attachment is obsolete: true
Attachment #341206 - Flags: review?(enndeakin)
Comment on attachment 341206 [details] [diff] [review]
Patch v.2

r+sr=mconnor, with changes discussed on IRC

(I'm going to say to land this since the /layout changes are basically just unhiding this stuff, if roc or someone wants to yell at me, they know where to find me)
Attachment #341206 - Flags: superreview+
Attachment #341206 - Flags: review?(enndeakin)
Attachment #341206 - Flags: review+
Attachment #341206 - Flags: review+
Comment on attachment 341206 [details] [diff] [review]
Patch v.2

r=bzbarsky on the forms.css changes.
The only reason the parentNode issue went away is because of DOM quickstubs, I bet.  If you want to depend on parentNode being quickstubbed and this code breaking if that ever changes, or if the behavior of quickstubs changes to be more backwards compatible in cases that happen to cover this one, then I guess removing nsISecurityCheckedComponent here is ok.
I guess I should clarify that until the state of quickstubs and XPConnect-less DOM access from JS in general is closer to being finalized, I would not support making that change.
Attached patch Patch v.3Splinter Review
Patch as landed (changeset 797db58b50f7).

* Addressed review comments
* Readded nsISecurityCheckedComponent stuff
* Tweaked image/css, as I noticed it had a sRGB/gAMA chunk in the PNG.

Tests will land in followups. Leaving this bug open for now, will close when I file followup bugs for tests and other parts (eg, scrubber) that didn't make this pass.
Attachment #341206 - Attachment is obsolete: true
Comment on attachment 341237 [details] [diff] [review]
Patch v.3

>+    <xbl:content xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
>+        <spacer flex="1"/>
>+        <stack xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">

Can the stack be removed?

>+            <hbox id="controlsBackground">
>+                <image id="playButton"
>+                       onclick="this.parentNode.parentNode.parentNode.Utils.togglePause();"/>
>+                <box id="controlsMiddle" flex="1"/>
>+                <image id="muteButton"
>+                       onclick="this.parentNode.parentNode.parentNode.Utils.toggleMute();"/>

You only want left clicks here, right?

>+                onMediaEvent : function (aEvent) {
>+                    this.log("Got " + aEvent.type + " media event");

>+                    return false;

The return value goes nowhere.

>+            function eventBouncer(aEvent) {
>+                self.Utils.onMediaEvent(aEvent);
>+            }
>+
>+            this.style.opacity = 0;
>+
>+            video.addEventListener("play",         eventBouncer, false);
>+            video.addEventListener("pause",        eventBouncer, false);
>+            video.addEventListener("volumechange", eventBouncer, false);

How about calling the method "handleEvent" instead of "onMediaEvent" and adding the event listeners like this:
video.addEventListener(..., this.Utils, false);
Is there any chance these same (basic) controls could be added for audio?
I get 

  JavaScript error: chrome://global/content/bindings/videocontrols.xml, line 179:
  self.Utils is undefined

a lot on the video at http://www.mozbox.org/jdll/video.xhtml
That error occurs anytime the video is removed from the DOM. eg:

var v = docuemnt.getElementsByTagName("video")[0]
v.parentNode.removeChild(v);

This will cause the error to occur. content/media/video/test/test_bug448534.html triggers this as well (It's currently commented out).
Um...  Are we relying on XBL-bound things to outlive removal from the DOM?  They generally don't have to.
I don't think we're relying on that. The controls are stateless --- or rather, all the state is inside the video element itself.
Well, there is currently some state --- whether the control bar is hidden or not, and how far through its fade animation is --- but we don't care whether that's preserved or not.
Well, except when init() is called (from the constructor) the binding adds event listeners.  These are never removed.  They reference a field of the binding.  If the node is removed from the DOM, the field goes away, while the event listeners stay, no?
I didn't see this mentioned after reading most comments:

- I am always interested in knowing how long a video is (is it 2 minutes or is it going to take 2 hours of my time?). Please always show the total time (at the end of the time slider).

- The volume control is almost as important as the time slider (1. Most video are ~2 minutes and are watched from beginning to end in one go. 2. Many videos have different volumes and I often find myself quickly needing to adjust the volume. 3. Most videos are short, so a slightly shorter time slider still provides enough control-resolution). Please provide an always visible horizontal volume slider (that is wide enough for good control, e.g. width: 2/3 of time slider) for one-click volume control.

- Make the whole video surface clickable for pause/play (Fitts's law)

- Make the time slider grippy larger, for easy mouse aiming (Fitts's law). Precision can be achieved via e.g., a horizontal oval with a precision marker at the top and bottom of the horizontal center of the oval. 

- if video is smaller than x pixels wide, only then switch to a two-click pop-up volume control

- Provide keyboard shortcuts for at least the most common actions:
  - cursor up/down    - increase/decrease volume
  - cursor left/right - wind video back/forward (by x seconds) (x=5?)
  - spacebar          - pause/resume video playback
  - <ENTER>/<ESC>     - enter/exit fullscreen mode
  - CTRL+[cursor keys]- increases the size of jumps in volume/winding
  - ALT +[cursor keys]- decreases the size of jumps in volume/winding

Mockup:

+----------------------------------------------------------+
| (>) [======(1:37)====__________]9:59  [<<<<<<(80%)__] (X)|
+----------------------------------------------------------+

- shows three "values": current position (1:37), total time (9:59), and volume in % (80%)

Legend:
 > is play (pause) button
 = is amount of video already downloaded
 _ is amount of video not yet downloaded / volume-increase still available
 < is amount of volume selected
 X is fullscreen button
This patch addresses the things Dao noted in comment 69.

I'm not sure how to address the issue bz noted in comment 76. The XBL destructor doesn't fire when the node's removed (bug 230086), so the binding has no way of knowing it should remove the event listeners.
Attachment #344773 - Flags: review?(mconnor)
One option is to have the listeners self-remove if they happen when the binding is not attached (instead of throwing as they do now).  That won't help if the node is repeatedly removed and reinserted, since in that case we'll effectively just allocate more and more listeners, but if they're idempotent the only real outcome will be that we use up more and more memory...

Fundamentally, of course, we shouldn't be using the sheltered beastie that is XBL out in the wild where it has to interact with arbitrary author script.  It's just asking for trouble.  :(
What is the best long-term plan then? Implement XBL2? Reimplement video controls in C++ in nsVideoFrame?
Long-term, probably xbl2...
Comment on attachment 344773 [details] [diff] [review]
Followup patch, v.1

Good catches Dao!
Attachment #344773 - Flags: review?(mconnor) → review+
Blocks: 462113
Ok, pushed the followup patch in changeset ce5e8d5a268d, and now closing this bug now like I said back in comment 68.

Followup work for the rest of the controls (progress indicator) will happen over in bug 462113. Tests are over in bug 462116, cleaning up the XBL event handlers in bug 462114, and controls for small media in bug 462117.

Further issues should go into new bugs or be redirected to other existing bugs.
No longer blocks: 462113
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
Blocks: TrackAVUI
Well, I can play around with the Pause/Play, Seekbar and Mute/UnMute buttons (and see video download progress) on Shiretoko and Trunk builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090414 Minefield/3.6a1pre ID:20090414030735

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090414 Shiretoko/3.5b4pre ID:20090414035212

Since there's some support for basic controls on html 5 video, I'm going to put this as verified FIXED unless someone has qualms about that.
Status: RESOLVED → VERIFIED
Flags: in-litmus?

Isn't this bug's title "Need *MORE* controls...", and not " Need ANY controls..."?

At a minimum, shouldn't this bug include a volume control? Or is there already another bug to add a volume control?

REOPEN?
(In reply to comment #85)
> Isn't this bug's title "Need *MORE* controls...", and not " Need ANY
> controls..."?
It got more controls from when this bug is filed.  The bug could never be closed with your logic, since we could always add more, right?

> At a minimum, shouldn't this bug include a volume control? Or is there already
> another bug to add a volume control?
> REOPEN?
No, you should file a new bug for that specific request if it doesn't already exist.
Found it. The bug to add a volume control is bug 475317
There's an Audio/Video FFT Litmus Subgroup in the Fx3.5 Testrun that addresses these tests.
Flags: in-litmus? → in-litmus+
Component: Video/Audio → Video/Audio Controls
Product: Core → Toolkit
QA Contact: video.audio → video.audio
Target Milestone: mozilla1.9.1b2 → mozilla1.9.2a1
Version: Trunk → unspecified
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b2
Nothing exists, AIUI, "with Flash". There are several licensed and free web based FLV players available such as:

http://hdwplayer.com
http://www.jwplayer.com
You need to log in before you can comment on or make changes to this bug.