Bug 64837 - [chromium] Media player controls do not fade out.
Summary: [chromium] Media player controls do not fade out.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Steve Lacey
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-19 16:38 PDT by Steve Lacey
Modified: 2011-07-19 22:52 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.13 KB, patch)
2011-07-19 17:01 PDT, Steve Lacey
no flags Details | Formatted Diff | Diff
Patch (4.13 KB, patch)
2011-07-19 17:27 PDT, Steve Lacey
no flags Details | Formatted Diff | Diff
Patch for landing (4.13 KB, patch)
2011-07-19 21:29 PDT, Steve Lacey
no flags Details | Formatted Diff | Diff
Patch for landing (4.14 KB, patch)
2011-07-19 22:40 PDT, Steve Lacey
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Lacey 2011-07-19 16:38:55 PDT
Due to drift between MediaControlRootElement and MediaControlRootElementChromium, the fading out of controls is broken. I have a fix for this, but post-this fix will look at sharing more code between the base and chromium implementations.
Comment 1 Steve Lacey 2011-07-19 17:01:21 PDT
Created attachment 101410 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2011-07-19 17:05:18 PDT
Comment on attachment 101410 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=101410&action=review

ew. I don't like code-copying. Can we do better?

> Source/WebCore/ChangeLog:9
> +        [chromium] Media player controls do not fade out.
> +        https://bugs.webkit.org/show_bug.cgi?id=64837

This should be on top.
Comment 3 Steve Lacey 2011-07-19 17:27:23 PDT
Created attachment 101412 [details]
Patch
Comment 4 Steve Lacey 2011-07-19 17:32:56 PDT
(In reply to comment #2)
> (From update of attachment 101410 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101410&action=review
> 
> ew. I don't like code-copying. Can we do better?

I agree, a bit icky, but this change is for the current chromium media controls, not the new ones. Currently the default root element implementation and the chromium implementation are very similar, after the split to create the MediaControls interface from a while ago.

Ideally there should probably be a base that handles common implementation, but I'd rather do that with the new media controls where there is a lot of differences - doing it now would cause more churn that'll end up changing with the new controls anyhow.

Although this isn't ideal, it does fix an unpleasant bug with the current chromium controls which I need to fix before the M14 cut.

I promise I'll make sure this is addressed for the new controls...

> 
> > Source/WebCore/ChangeLog:9
> > +        [chromium] Media player controls do not fade out.
> > +        https://bugs.webkit.org/show_bug.cgi?id=64837
> 
> This should be on top.

Fixed.
Comment 5 Steve Lacey 2011-07-19 21:29:30 PDT
Created attachment 101425 [details]
Patch for landing
Comment 6 WebKit Review Bot 2011-07-19 22:30:40 PDT
Comment on attachment 101425 [details]
Patch for landing

Rejecting attachment 101425 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1

Last 500 characters of output:
d81223d29e6fe8fece485860533c0860185c26cc
r91334 = 2ad3711cc4535916954b89606e83af9757b7e091
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9193134
Comment 7 Steve Lacey 2011-07-19 22:40:58 PDT
Created attachment 101427 [details]
Patch for landing
Comment 8 WebKit Review Bot 2011-07-19 22:52:39 PDT
Comment on attachment 101427 [details]
Patch for landing

Clearing flags on attachment: 101427

Committed r91337: <http://trac.webkit.org/changeset/91337>
Comment 9 WebKit Review Bot 2011-07-19 22:52:43 PDT
All reviewed patches have been landed.  Closing bug.