Bug 52822

Summary: [Qt] Clean up the Media Controls CSS for Qt
Product: WebKit Reporter: Yi Shen <max.hong.shen>
Component: CSSAssignee: Yi Shen <max.hong.shen>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, commit-queue, eric.carlson, eric, keith.rosenblatt, nancy.piedra
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 51543    
Attachments:
Description Flags
first try none

Description Yi Shen 2011-01-20 10:40:14 PST
As discussed in https://bugs.webkit.org/show_bug.cgi?id=52315, need to clean up the mediaControlsQt.css by splitting the audio::-webkit-media-xxx and video::-webkit-media-xxx rules, and remove the duplicate audio::-webkit-media-xxx rules from mediaControlsQtFullscreen.css.
Comment 1 Yi Shen 2011-01-20 10:44:14 PST
Created attachment 79616 [details]
first try
Comment 2 Eric Carlson 2011-01-20 11:00:19 PST
Comment on attachment 79616 [details]
first try

View in context: https://bugs.webkit.org/attachment.cgi?id=79616&action=review

Marking r+ but cq- as it doesn't look like this is quite right (or I am not looking at it correctly ;-) )

> Source/WebCore/css/mediaControlsQtFullscreen.css:62
> +video::-webkit-media-controls-time-remaining-display {
>      display: none;
>  }

Does this change anything?

> Source/WebCore/css/mediaControlsQtFullscreen.css:80
> +video::-webkit-media-controls-seek-back-button {
>      display: none;
>  }

Or this?

> Source/WebCore/css/mediaControlsQtFullscreen.css:84
> +video::-webkit-media-controls-seek-forward-button {
>      display: none;
>  }

Ditto.

> Source/WebCore/css/mediaControlsQtFullscreen.css:92
> +video::-webkit-media-controls-rewind-button {
>      display: none;
>  }

Ditto.

> Source/WebCore/css/mediaControlsQtFullscreen.css:96
> +video::-webkit-media-controls-return-to-realtime-button {
>      display: none;
>  }

Ditto.

> Source/WebCore/css/mediaControlsQtFullscreen.css:100
> +video::-webkit-media-controls-toggle-closed-captions-button {
>      display: none;
>  }

Ditto.
Comment 3 Yi Shen 2011-01-20 11:21:06 PST
(In reply to comment #2)
> (From update of attachment 79616 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79616&action=review
> 
> Marking r+ but cq- as it doesn't look like this is quite right (or I am not looking at it correctly ;-) )
> 
> > Source/WebCore/css/mediaControlsQtFullscreen.css:62
> > +video::-webkit-media-controls-time-remaining-display {
> >      display: none;
> >  }
> 
> Does this change anything?
> 
> > Source/WebCore/css/mediaControlsQtFullscreen.css:80
> > +video::-webkit-media-controls-seek-back-button {
> >      display: none;
> >  }
> 
> Or this?
> 
> > Source/WebCore/css/mediaControlsQtFullscreen.css:84
> > +video::-webkit-media-controls-seek-forward-button {
> >      display: none;
> >  }
> 
> Ditto.
> 
> > Source/WebCore/css/mediaControlsQtFullscreen.css:92
> > +video::-webkit-media-controls-rewind-button {
> >      display: none;
> >  }
> 
> Ditto.
> 
> > Source/WebCore/css/mediaControlsQtFullscreen.css:96
> > +video::-webkit-media-controls-return-to-realtime-button {
> >      display: none;
> >  }
> 
> Ditto.
> 
> > Source/WebCore/css/mediaControlsQtFullscreen.css:100
> > +video::-webkit-media-controls-toggle-closed-captions-button {
> >      display: none;
> >  }
> 
> Ditto.

Thanks for reviewing, Eric :) I don't think these rules changed anything (they are exactly the same as in mediaControlsQt.css). I preserve these rules in mediaControlsQtFullscreen.css, so that it won't be affected if someone changes the style for video in mediaControlsQt.css.

If you don't like them, I can remove those from mediaControlsQtFullscreen.css :)
Comment 4 Eric Carlson 2011-01-20 12:09:13 PST
(In reply to comment #3)
> Thanks for reviewing, Eric :) I don't think these rules changed anything (they are exactly the same as in mediaControlsQt.css). I preserve these rules in mediaControlsQtFullscreen.css, so that it won't be affected if someone changes the style for video in mediaControlsQt.css.
> 
> If you don't like them, I can remove those from mediaControlsQtFullscreen.css :)

Right, my point was that they don't do anything and therefore aren't needed. I marked r+ so you can submit as-is if you want.
Comment 5 Yi Shen 2011-01-20 12:13:09 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Thanks for reviewing, Eric :) I don't think these rules changed anything (they are exactly the same as in mediaControlsQt.css). I preserve these rules in mediaControlsQtFullscreen.css, so that it won't be affected if someone changes the style for video in mediaControlsQt.css.
> > 
> > If you don't like them, I can remove those from mediaControlsQtFullscreen.css :)
> 
> Right, my point was that they don't do anything and therefore aren't needed. I marked r+ so you can submit as-is if you want.

Thanks :-)
Comment 6 WebKit Commit Bot 2011-01-20 12:41:47 PST
Comment on attachment 79616 [details]
first try

Clearing flags on attachment: 79616

Committed r76272: <http://trac.webkit.org/changeset/76272>
Comment 7 WebKit Commit Bot 2011-01-20 12:41:53 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Keith Rosenblatt 2011-01-21 11:52:55 PST
*** Bug 52474 has been marked as a duplicate of this bug. ***
Comment 9 Ademar Reis 2011-01-28 11:55:59 PST
Revision r76272 cherry-picked into qtwebkit-2.1.x with commit 7623b93 <http://gitorious.org/webkit/qtwebkit/commit/7623b93>