Bug 145827 – Update media controls JS and CSS to use pip

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jon Lee]

Update media controls JS and CSS to use pip

[Radar WebKit Bug Importer]

<rdar://problem/21311576>

[Jon Lee]

Created attachment 254737 [details]
Simplify event handling logic for search cancel button

[Jon Lee]

Comment on attachment 254737 [details]
Simplify event handling logic for search cancel button

ignore that attachment. bad passing of arguments in webkit-patch

[Jon Lee]

Created attachment 254741 [details]
Need a short description (OOPS!).

[Jon Lee]

Created attachment 254742 [details]
Replace gSimulateOptimizedFullscreenAvailable

[Jon Lee]

Created attachment 254743 [details]
Replace optimizedFullscreenButton

[Jon Lee]

Created attachment 254744 [details]
Replace handleOptimizedFullscreenButtonClicked

[Jon Lee]

Created attachment 254745 [details]
Replace pseudo -webkit-media-controls-optimized-fullscreen-button

[Jon Lee]

Created attachment 254746 [details]
Move images and styles into js and css

[Jon Lee]

Created attachment 254747 [details]
Update handle events

[Jon Lee]

Created attachment 254748 [details]
Clean up MediaControlsHost and HTMLMediaElement

[Jon Lee]

Created attachment 254749 [details]
Need a short description (OOPS!).

[Jon Lee]

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.

[Jon Lee]

Created attachment 254755 [details]
Patch for submission

[Dean Jackson]

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?

[Dean Jackson]

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.

[Jon Lee]

Committed r185472: <http://trac.webkit.org/changeset/185472>

[Jon Lee]

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

[Eric Carlson]

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.