Bug 145827

Summary: Update media controls JS and CSS to use pip
Product: WebKit Reporter: Jon Lee <jonlee>
Component: MediaAssignee: Jon Lee <jonlee>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 145825    
Attachments:
Description Flags
Simplify event handling logic for search cancel button
none
Need a short description (OOPS!).
none
Replace gSimulateOptimizedFullscreenAvailable
dino: review+
Replace optimizedFullscreenButton
dino: review+
Replace handleOptimizedFullscreenButtonClicked
dino: review+
Replace pseudo -webkit-media-controls-optimized-fullscreen-button
dino: review+
Move images and styles into js and css
dino: review+
Update handle events
dino: review+
Clean up MediaControlsHost and HTMLMediaElement
dino: review+
Need a short description (OOPS!).
dino: review+
Patch for submission dino: review+

Description Jon Lee 2015-06-09 16:39:08 PDT
Update media controls JS and CSS to use pip
Comment 1 Radar WebKit Bug Importer 2015-06-09 16:39:22 PDT
<rdar://problem/21311576>
Comment 2 Jon Lee 2015-06-11 12:07:05 PDT
Created attachment 254737 [details]
Simplify event handling logic for search cancel button
Comment 3 Jon Lee 2015-06-11 12:07:47 PDT
Comment on attachment 254737 [details]
Simplify event handling logic for search cancel button

ignore that attachment. bad passing of arguments in webkit-patch
Comment 4 Jon Lee 2015-06-11 12:38:16 PDT
Created attachment 254741 [details]
Need a short description (OOPS!).
Comment 5 Jon Lee 2015-06-11 12:38:47 PDT
Created attachment 254742 [details]
Replace gSimulateOptimizedFullscreenAvailable
Comment 6 Jon Lee 2015-06-11 12:38:48 PDT
Created attachment 254743 [details]
Replace optimizedFullscreenButton
Comment 7 Jon Lee 2015-06-11 12:38:50 PDT
Created attachment 254744 [details]
Replace handleOptimizedFullscreenButtonClicked
Comment 8 Jon Lee 2015-06-11 12:38:52 PDT
Created attachment 254745 [details]
Replace pseudo -webkit-media-controls-optimized-fullscreen-button
Comment 9 Jon Lee 2015-06-11 12:38:54 PDT
Created attachment 254746 [details]
Move images and styles into js and css
Comment 10 Jon Lee 2015-06-11 12:38:56 PDT
Created attachment 254747 [details]
Update handle events
Comment 11 Jon Lee 2015-06-11 12:38:59 PDT
Created attachment 254748 [details]
Clean up MediaControlsHost and HTMLMediaElement
Comment 12 Jon Lee 2015-06-11 12:40:19 PDT
Created attachment 254749 [details]
Need a short description (OOPS!).
Comment 13 Jon Lee 2015-06-11 13:21:37 PDT
I've upload 8 small patches for review. I will also post the full patch that EWS can chew on and will represent what I will submit.
Comment 14 Jon Lee 2015-06-11 13:42:22 PDT
Created attachment 254755 [details]
Patch for submission
Comment 15 Dean Jackson 2015-06-11 13:46:02 PDT
Comment on attachment 254743 [details]
Replace optimizedFullscreenButton

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

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:468
> +        pictureInPictureButton.setAttribute('pseudo', '-webkit-media-controls-optimized-fullscreen-button');

This pseudo attribute should change too.

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:469
> +        pictureInPictureButton.setAttribute('aria-label', this.UIString('Display Optimized Full Screen'));

And maybe this label.

> Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js:130
> +        this.listenFor(this.controls.pictureInPictureButton, 'touchstart', this.handleOptimizedFullscreenTouchStart);
> +        this.listenFor(this.controls.pictureInPictureButton, 'touchend', this.handleOptimizedFullscreenTouchEnd);
> +        this.listenFor(this.controls.pictureInPictureButton, 'touchcancel', this.handleOptimizedFullscreenTouchCancel);

Are we changing the function names in a later patch?
Comment 16 Dean Jackson 2015-06-11 13:50:23 PDT
Comment on attachment 254755 [details]
Patch for submission

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

> Source/WebCore/ChangeLog:3
> +        Update media controls JS and CSS to use pip

I think we should expand "pip" in the bug title.
Comment 17 Jon Lee 2015-06-11 13:54:23 PDT
Committed r185472: <http://trac.webkit.org/changeset/185472>
Comment 18 Jon Lee 2015-06-11 13:55:20 PDT
(In reply to comment #16)
> Comment on attachment 254755 [details]
> Patch for submission
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254755&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Update media controls JS and CSS to use pip
> 
> I think we should expand "pip" in the bug title.

Updated in the ChangeLogs.
Comment 19 Eric Carlson 2015-06-11 14:07:09 PDT
Comment on attachment 254755 [details]
Patch for submission

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

>>> Source/WebCore/ChangeLog:3
>>> +        Update media controls JS and CSS to use pip
>> 
>> I think we should expand "pip" in the bug title.
> 
> Updated in the ChangeLogs.

NitL "PiP" would be more correct.