Bug 52822

Summary:

[Qt] Clean up the Media Controls CSS for Qt

Product:

WebKit

Reporter:

Yi Shen <max.hong.shen>

Component:

CSS

Assignee:

Yi Shen <max.hong.shen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ademar, commit-queue, eric.carlson, eric, keith.rosenblatt, nancy.piedra

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Bug Depends on:

Bug Blocks:

51543

Attachments:

Description	Flags
first try	none

Yi Shen

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

Attachments
first try (12.99 KB, patch) 2011-01-20 10:44 PST, Yi Shen	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Yi Shen

Comment 1 2011-01-20 10:44:14 PST

Created attachment 79616 [details] first try

Eric Carlson

Comment 2 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.

Yi Shen

Comment 3 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 :)

Eric Carlson

Comment 4 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.

Yi Shen

Comment 5 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 :-)

WebKit Commit Bot

Comment 6 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>

WebKit Commit Bot

Comment 7 2011-01-20 12:41:53 PST

All reviewed patches have been landed. Closing bug.

Keith Rosenblatt

Comment 8 2011-01-21 11:52:55 PST

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

Ademar Reis

Comment 9 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>

Note You need to log in before you can comment on or make changes to this bug.