Bug 88743

Summary: Change mute button and volume slider behaviour for new Chrome video controls
Product: WebKit Reporter: Silvia Pfeiffer <silviapf>
Component: MediaAssignee: Silvia Pfeiffer <silviapf>
Status: RESOLVED FIXED    
Severity: Normal CC: annacc, cmarcelo, dglazkov, eric.carlson, eric, feature-media-reviews, macpherson, menard, scherkus, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 87683, 87835, 88297, 88623, 88724    
Bug Blocks: 84672, 88818, 89050    
Attachments:
Description Flags
Full patch for layout tests
none
Use this for review
none
Full patch for layout tests - with unmute name change
none
Patch for review with unmute name change.
none
patch for cq none

Silvia Pfeiffer
Reported 2012-06-10 19:29:30 PDT
This is the sixth patch for the introduction of the new Chromium video controls, master bug at https://bugs.webkit.org/show_bug.cgi?id=84672 . The new controls require that when the user clicks the mute button, the slider thumb should animate to 0% and restore the previous volume level when clicking the mute button again. Further, when the user drags the thumb or clicks on the slider, the video unmutes and sets the volume to the user's chosen level.
Attachments
Full patch for layout tests (78.61 KB, patch)
2012-06-10 19:56 PDT, Silvia Pfeiffer 		no flags
DetailsFormatted DiffDiff
Use this for review (5.82 KB, patch)
2012-06-10 19:58 PDT, Silvia Pfeiffer 		no flags
DetailsFormatted DiffDiff
Full patch for layout tests - with unmute name change (78.84 KB, patch)
2012-06-10 22:32 PDT, Silvia Pfeiffer 		no flags
DetailsFormatted DiffDiff
Patch for review with unmute name change. (6.01 KB, patch)
2012-06-10 22:34 PDT, Silvia Pfeiffer 		no flags
DetailsFormatted DiffDiff
patch for cq (6.37 KB, patch)
2012-06-13 17:51 PDT, Silvia Pfeiffer 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Silvia Pfeiffer
Comment 1 2012-06-10 19:56:38 PDT
Created attachment 146765 [details] Full patch for layout tests
Silvia Pfeiffer
Comment 2 2012-06-10 19:58:36 PDT
Created attachment 146766 [details] Use this for review This is the difference to the previous patch. It will not pass the test - refer to the full patch for that.
Eric Carlson
Comment 3 2012-06-10 20:19:27 PDT
Comment on attachment 146766 [details] Use this for review View in context: https://bugs.webkit.org/attachment.cgi?id=146766&action=review > Source/WebCore/html/shadow/MediaControlElements.cpp:904 > + , m_unmute(false) "unmute" is a confusing name. The flag means "set muted to false on user interaction", so maybe something like "m_clearMutedOnUserInteraction"? > Source/WebCore/html/shadow/MediaControlElements.cpp:948 > +void MediaControlVolumeSliderElement::setUnmuteOnInteraction(bool unmute) Ditto for "setUnmute"
Silvia Pfeiffer
Comment 4 2012-06-10 21:25:54 PDT
(In reply to comment #3) > (From update of attachment 146766 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146766&action=review > > > Source/WebCore/html/shadow/MediaControlElements.cpp:904 > > + , m_unmute(false) > > "unmute" is a confusing name. The flag means "set muted to false on user interaction", so maybe something like "m_clearMutedOnUserInteraction"? > > > Source/WebCore/html/shadow/MediaControlElements.cpp:948 > > +void MediaControlVolumeSliderElement::setUnmuteOnInteraction(bool unmute) > > Ditto for "setUnmute" Good ideas, DONE.
Silvia Pfeiffer
Comment 5 2012-06-10 22:32:27 PDT
Created attachment 146784 [details] Full patch for layout tests - with unmute name change
Silvia Pfeiffer
Comment 6 2012-06-10 22:34:28 PDT
Created attachment 146785 [details] Patch for review with unmute name change. Patch for review with unmute name change.
WebKit Review Bot
Comment 7 2012-06-10 23:33:17 PDT
Comment on attachment 146784 [details] Full patch for layout tests - with unmute name change Attachment 146784 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12948057
Silvia Pfeiffer
Comment 8 2012-06-13 17:51:21 PDT
Created attachment 147451 [details] patch for cq
WebKit Review Bot
Comment 9 2012-06-14 10:00:55 PDT
Comment on attachment 147451 [details] patch for cq Clearing flags on attachment: 147451 Committed r120337: <http://trac.webkit.org/changeset/120337>
WebKit Review Bot
Comment 10 2012-06-14 10:01:02 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.