Bug 88297 – Change the styling of the Chromium video controls

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Silvia Pfeiffer]

This is a third patch for the introduction of the new Chromium video controls, master bug at https://bugs.webkit.org/show_bug.cgi?id=84672 .

This does the actual visual update. As such, it includes the change of the CSS using the new flexbox and new styles, the removal of the now not needed controls background div, and the introduction of the new images for the buttons.

The visual update of the range sliders will be in a separate patch.

Not to land separately and to land after 87835.

[Silvia Pfeiffer]

Created attachment 145954 [details]
Full patch for layout tests

[Silvia Pfeiffer]

Created attachment 145956 [details]
Review this for the differences to the second patch

As it turns out, there are still too many changes to the new flexbox model, so I've gone back to the old one, which is also more consistent with the other WebKit styles for video controls shadow DOM elements.

[Eric Carlson]

Comment on attachment 145956 [details]
Review this for the differences to the second patch

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

> Source/WebCore/rendering/RenderMediaControlsChromium.cpp:278
> +const int mediaSliderThumbWidth = 32;
> +const int mediaSliderThumbHeight = 24;

Nit: you might want to add "Timeline" to these names to match the pattern used for the other names.

[Silvia Pfeiffer]

(In reply to comment #3)
> (From update of attachment 145956 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145956&action=review
> 
> > Source/WebCore/rendering/RenderMediaControlsChromium.cpp:278
> > +const int mediaSliderThumbWidth = 32;
> > +const int mediaSliderThumbHeight = 24;
> 
> Nit: you might want to add "Timeline" to these names to match the pattern used for the other names.

Thanks, will do when uploading the patch for landing rebased to the current codebase.

[Silvia Pfeiffer]

(In reply to comment #3)
> (From update of attachment 145956 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145956&action=review
> 
> > Source/WebCore/rendering/RenderMediaControlsChromium.cpp:278
> > +const int mediaSliderThumbWidth = 32;
> > +const int mediaSliderThumbHeight = 24;
> 
> Nit: you might want to add "Timeline" to these names to match the pattern used for the other names.

Decided not to do this for consistency, since all constants and variable names in the media controls system for the timeline/transport are called "mediaslider".

[Silvia Pfeiffer]

Created attachment 147185 [details]
Patch for cq

[WebKit Review Bot]

Comment on attachment 147185 [details]
Patch for cq

Clearing flags on attachment: 147185

Committed r120171: <http://trac.webkit.org/changeset/120171>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.