850458 - work - add full screen video element to html5 video controls

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect, P2)

Description

[juan becerra [:juanb]]

Tested on 2013-03-12 on nightly built from http://hg.mozilla.org/mozilla-central/rev/7433bc4545c9

While regression testing 801183, I found myself wanting a fullscreen video control in the html5 video I tried.

Steps:
1. Go to http://clips.vorwaerts-gmbh.de/big_buck_bunny.ogv
2. Try to fullscreen the video

Expected: Some control to fullscreen the video playback

Actual: No such control element.

Comment 1

[Asa Dotzler [:asa]]

It is important to get full screen for HTML video in these controls because we won't have Flash support and the video tag is going to be the primary video experience on Metro.  Also, is this bug materially different from bug 842130?

Jared, any change you have some cycles to help us with this?

Comment 2

[Brian R. Bondy [:bbondy]]

Should we make the html5 video control the same as Desktop Firefox?

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Brian R. Bondy [:bbondy] from comment #2)
> Should we make the html5 video control the same as Desktop Firefox?

I haven't gotten the chance to look yet (it's on my todo list), but I think metro uses mobile's video controls. Mobile and desktop should have a fullscreen button already in the video controls, so something must not be showing it.

I'll try to look into this later today.

Comment 4

[Asa Dotzler [:asa]]

(In reply to Brian R. Bondy [:bbondy] from comment #2)
> Should we make the html5 video control the same as Desktop Firefox?

Are our desktop controls touch friendly? If so, let's use them. If not, let's figure out how to augment what we have that is touch friendly.

Comment 5

[Brian R. Bondy [:bbondy]]

Probably not 'as' touch friendly. OK.

Comment 6

[Brian R. Bondy [:bbondy]]

Attachment: Screenshot of Metro video controls

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Brian R. Bondy [:bbondy] from comment #6)
> Created attachment 730349 [details]
> Screenshot of Metro video controls

Thanks! I was just firing up my VM. Those are the touch controls, designed to be touch-friendly.

Ok, this is the problem here:
http://mxr.mozilla.org/mozilla-central/source/browser/metro/theme/touchcontrols.css#56

You can see what b2g did: http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/touchcontrols.css#30

B2G has graphics for it, but they might be slightly different (they are newer though).

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

BTW, I don't think I'll have time to help with fixing this bug. Hopefully comment #7 is enough of a guide though. I'll try to respond to questions though.

Comment 9

[Brian R. Bondy [:bbondy]]

Thanks for the info Jared!

Comment 11

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Basically as per your suggestions in the other dupe bug and Jared's suggestion here I just made the css consistent with B2G and everything worked.

Comment 12

[Brian R. Bondy [:bbondy]]

*and added the 2 missing images and modified the image paths but that's it.

Comment 13

[Matt Brubeck (:mbrubeck)]

Comment on attachment 730863 [details] [diff] [review]
Patch v1.

I'm also following bug 704229 which should allow us to share all this code with Fennec and B2G.

Comment 14

[Jim Mathies [:jimm]]

We still have all the xul fennec fullscreen code sitting in the repo. Deciding what to do with that was bug 842130. If we're going to dupe over, we'll need to figure out what we plan to do with all of this code here.

http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/video.js
http://mxr.mozilla.org/mozilla-central/source/browser/metro/modules/video.jsm
http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/pages/fullscreen-video.xhtml

Comment 15

[Brian R. Bondy [:bbondy]]

OK I'll re-open it for that.

Comment 16

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b420dcf17910

Comment 17

[Brian R. Bondy [:bbondy]]

Backout for potential bustage (on some slaves but not others :S)
https://hg.mozilla.org/integration/mozilla-inbound/rev/088d73e55949

Comment 18

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b35998de0fe

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8b35998de0fe