Bug 52315

Summary: [Qt] Use a big central media play control button for video element
Product: WebKit Reporter: Yi Shen <max.hong.shen>
Component: WebKit QtAssignee: Yi Shen <max.hong.shen>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, commit-queue, eric.carlson, eric, hausmann, jer.noble, nancy.piedra
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
first try
none
screenshot none

Description Yi Shen 2011-01-12 11:41:10 PST 
When chrome::requiresFullscreenForVideoPlayback() returns true, all other control buttons for the video element should be hidden, except the play button. When user clicks on the big centralized play button, the video starts playing.
Comment 1 Yi Shen 2011-01-12 11:56:10 PST 
Created attachment 78716 [details]
first try
Comment 2 Eric Seidel (no email) 2011-01-12 12:03:08 PST 
Looks reasonable to me.  CCing video folks in case they want to comment.
Comment 3 Eric Seidel (no email) 2011-01-12 12:04:28 PST 
Comment on attachment 78716 [details]
first try

I don't really know what these controls are supposed to look like, so all I can do is rubber stamp this (which I'm willing to do).  If you have concerns as to if you got the CSS "right" or not, you should ask one of the video folks I CC'd.
Comment 4 Eric Carlson 2011-01-12 12:14:02 PST 
Why don't you split the audio::-webkit-media-xxx and video::-webkit-media-xxx rules in mediaControlsQt.css so you don't have to include duplicate audio::-webkit-media-xxx here? 

As you have it now, you will need to keep mediaControlsQt.css and mediaControlsQtFullscreen.css in sync whenever you change the style of an <audio> element.
Comment 5 Yi Shen 2011-01-12 12:17:01 PST 
Created attachment 78719 [details]
screenshot
Comment 6 Yi Shen 2011-01-12 12:23:23 PST 
(In reply to comment #4)
> Why don't you split the audio::-webkit-media-xxx and video::-webkit-media-xxx rules in mediaControlsQt.css so you don't have to include duplicate audio::-webkit-media-xxx here? 
> 
> As you have it now, you will need to keep mediaControlsQt.css and mediaControlsQtFullscreen.css in sync whenever you change the style of an <audio> element.

Thanks for reviewing, Eric. I think you are right, But if you don't mind, I will split it in a separate bug when I got time (just after I finished bug 51248, which has the highest priority for me). Thanks again :)
Comment 7 Eric Seidel (no email) 2011-01-12 12:23:45 PST 
Comment on attachment 78716 [details]
first try

removing from the cq to allow further discussion.
Comment 8 Yi Shen 2011-01-12 12:24:26 PST 
(In reply to comment #6)
> (In reply to comment #4)
> > Why don't you split the audio::-webkit-media-xxx and video::-webkit-media-xxx rules in mediaControlsQt.css so you don't have to include duplicate audio::-webkit-media-xxx here? 
> > 
> > As you have it now, you will need to keep mediaControlsQt.css and mediaControlsQtFullscreen.css in sync whenever you change the style of an <audio> element.
> 
> Thanks for reviewing, Eric. I think you are right, But if you don't mind, I will split it in a separate bug when I got time (just after I finished bug 51248, which has the highest priority for me). Thanks again :)

Sorry, I mean bug 51249.
Comment 9 Andreas Kling 2011-01-13 06:51:22 PST 
Comment on attachment 78716 [details]
first try

cq=me, Yi will clean up the CSS later on :)
Comment 10 WebKit Commit Bot 2011-01-13 07:12:21 PST 
Comment on attachment 78716 [details]
first try

Clearing flags on attachment: 78716

Committed r75711: <http://trac.webkit.org/changeset/75711>
Comment 11 WebKit Commit Bot 2011-01-13 07:12:28 PST 
All reviewed patches have been landed.  Closing bug.
Comment 12 Ademar Reis 2011-01-13 11:06:15 PST 
Revision r75711 cherry-picked into qtwebkit-2.2 with commit ff08dd9 <http://gitorious.org/webkit/qtwebkit/commit/ff08dd9>