Bug 30529

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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Round 1	levin: review-
Round 2	none

Andrew Scherkus

Reported 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.

Attachments
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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Andrew Scherkus

Comment 1 2009-10-19 12:53:38 PDT

Created attachment 41440 [details] Round 1

Eric Seidel (no email)

Comment 2 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"?

Eric Seidel (no email)

Comment 3 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?

Andrew Scherkus

Comment 4 2009-10-19 13:40:20 PDT

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

David Levin

Comment 5 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.

Andrew Scherkus

Comment 6 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!

Eric Seidel (no email)

Comment 7 2009-10-20 11:59:20 PDT

Comment on attachment 41449 [details] Round 2 LGTM.

WebKit Commit Bot

Comment 8 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>

WebKit Commit Bot

Comment 9 2009-10-20 12:08:31 PDT

All reviewed patches have been landed. Closing bug.

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