Bug 30529 – Hide Chromium's media slider thumb if no source has been loaded.

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

[Andrew Scherkus]

Since the slider is non-interactive, it does not make sense to display an active-appearing slider thumb.

[Andrew Scherkus]

Created attachment 41440 [details]
Round 1

[Eric Seidel (no email)]

Comment on attachment 41440 [details]
Round 1

It is much better to list the layout tests rather than just saying "there are some".  Eventually all of chromiums tests will be upstream and we won't have untestable patches like this. :(

I thought "false" here meant "no theme-specific painting, webcore should paint if it want to"?

[Eric Seidel (no email)]

Comment on attachment 41440 [details]
Round 1

It seems there was at least one other:
if (mediaElement->networkState() == HTMLMediaElement::NETWORK_EMPTY ||
 154         mediaElement->networkState() == HTMLMediaElement::NETWORK_NO_SOURCE)
check, maybe this check should be unified into a static method which can be used in multiple places in this file?

[Andrew Scherkus]

I like the suggestions -- I'll upload something new.

[David Levin]

Comment on attachment 41440 [details]
Round 1

r- to move out of the queue since Andrew said he is uploading something new.


> diff --git a/WebCore/rendering/RenderMediaControlsChromium.cpp b/WebCore/rendering/RenderMediaControlsChromium.cpp
> +    if (mediaElement->networkState() == HTMLMediaElement::NETWORK_EMPTY ||
> +        mediaElement->networkState() == HTMLMediaElement::NETWORK_NO_SOURCE)
fyi, the || should be on the next line.

[Andrew Scherkus]

Created attachment 41449 [details]
Round 2

Here ya go!

P.S. I can't wait for the layout tests to be in WebKit... doing two-sided change + rebaselines are killing me!

[Eric Seidel (no email)]

Comment on attachment 41449 [details]
Round 2

LGTM.

[WebKit Commit Bot]

Comment on attachment 41449 [details]
Round 2

Clearing flags on attachment: 41449

Committed r49878: <http://trac.webkit.org/changeset/49878>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.