Bug 30529 - Hide Chromium's media slider thumb if no source has been loaded.
: Hide Chromium's media slider thumb if no source has been loaded.
Product: WebKit
Classification: Unclassified
Component: Media Elements
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Nobody
Depends on:
  Show dependency treegraph
Reported: 2009-10-19 12:51 PDT by Andrew Scherkus
Modified: 2009-10-20 12:08 PDT (History)
2 users (show)

See Also:

Round 1 (1.52 KB, patch)
2009-10-19 12:53 PDT, Andrew Scherkus
levin: review-
Details | Formatted Diff | Diff
Round 2 (3.41 KB, patch)
2009-10-19 14:17 PDT, Andrew Scherkus
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Scherkus 2009-10-19 12:51:12 PDT
Since the slider is non-interactive, it does not make sense to display an active-appearing slider thumb.
Comment 1 Andrew Scherkus 2009-10-19 12:53:38 PDT
Created attachment 41440 [details]
Round 1
Comment 2 Eric Seidel 2009-10-19 13:37:01 PDT
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"?
Comment 3 Eric Seidel 2009-10-19 13:37:51 PDT
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?
Comment 4 Andrew Scherkus 2009-10-19 13:40:20 PDT
I like the suggestions -- I'll upload something new.
Comment 5 David Levin 2009-10-19 14:02:16 PDT
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.
Comment 6 Andrew Scherkus 2009-10-19 14:17:49 PDT
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!
Comment 7 Eric Seidel 2009-10-20 11:59:20 PDT
Comment on attachment 41449 [details]
Round 2

Comment 8 WebKit Commit Bot 2009-10-20 12:08:27 PDT
Comment on attachment 41449 [details]
Round 2

Clearing flags on attachment: 41449

Committed r49878: <http://trac.webkit.org/changeset/49878>
Comment 9 WebKit Commit Bot 2009-10-20 12:08:31 PDT
All reviewed patches have been landed.  Closing bug.