Bug 64837

Summary: [chromium] Media player controls do not fade out.
Product: WebKit Reporter: Steve Lacey <sjl>
Component: MediaAssignee: Steve Lacey <sjl>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric.carlson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

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.