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.
Status: RESOLVED FIXED
: WebKit
Media Elements
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-10-19 12:51 PST by
Modified: 2009-10-20 12:08 PST (History)


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


Note

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


Description From 2009-10-19 12:51:12 PST
Since the slider is non-interactive, it does not make sense to display an active-appearing slider thumb.
------- Comment #1 From 2009-10-19 12:53:38 PST -------
Created an attachment (id=41440) [details]
Round 1
------- Comment #2 From 2009-10-19 13:37:01 PST -------
(From update of attachment 41440 [details])
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 From 2009-10-19 13:37:51 PST -------
(From update of attachment 41440 [details])
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 From 2009-10-19 13:40:20 PST -------
I like the suggestions -- I'll upload something new.
------- Comment #5 From 2009-10-19 14:02:16 PST -------
(From update of attachment 41440 [details])
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 From 2009-10-19 14:17:49 PST -------
Created an attachment (id=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 From 2009-10-20 11:59:20 PST -------
(From update of attachment 41449 [details])
LGTM.
------- Comment #8 From 2009-10-20 12:08:27 PST -------
(From update of attachment 41449 [details])
Clearing flags on attachment: 41449

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