Bug 52822 – [Qt] Clean up the Media Controls CSS for Qt

• Component: CSS
• Status: RESOLVED FIXED

[Yi Shen]

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.

[Yi Shen]

Created attachment 79616 [details]
first try

[Eric Carlson]

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.

[Yi Shen]

(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 :)

[Eric Carlson]

(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.

[Yi Shen]

(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 :-)

[WebKit Commit Bot]

Comment on attachment 79616 [details]
first try

Clearing flags on attachment: 79616

Committed r76272: <http://trac.webkit.org/changeset/76272>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Keith Rosenblatt]

*** Bug 52474 has been marked as a duplicate of this bug. ***

[Ademar Reis]

Revision r76272 cherry-picked into qtwebkit-2.1.x with commit 7623b93 <http://gitorious.org/webkit/qtwebkit/commit/7623b93>