Summary: | Hide Chromium's media slider thumb if no source has been loaded. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andrew Scherkus <scherkus> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric.carlson | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Andrew Scherkus
2009-10-19 12:51:12 PDT
Created attachment 41440 [details]
Round 1
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 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?
I like the suggestions -- I'll upload something new. 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. 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 on attachment 41449 [details]
Round 2
LGTM.
Comment on attachment 41449 [details] Round 2 Clearing flags on attachment: 41449 Committed r49878: <http://trac.webkit.org/changeset/49878> All reviewed patches have been landed. Closing bug. |