Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PiP pollyfill #1902

Merged
merged 11 commits into from May 3, 2019

Conversation

Projects
None yet
3 participants
@avelad
Copy link
Contributor

commented Apr 29, 2019

Add PiP pollyfill for Safari

Based in https://github.com/gbentaieb/pip-polyfill

@avelad

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

Currently the PR has errors, but I need some guidance for resolve it.

[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Generating Closure dependencies...
[INFO] Linting JavaScript...
[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Linting HTML...
[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Checking that the build files are complete...
[INFO] Checking the tests for type errors...
[WARNING] No changes detected, skipping. Use --force to override.
/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:57: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
if (this.webkitPresentationMode &&
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:57: ERROR - dangerous use of the global this object
if (this.webkitPresentationMode &&
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:58: ERROR - dangerous use of the global this object
this.webkitPresentationMode !== 'picture-in-picture') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:65: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
if (!this.$pictureInPictureElement) {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:71: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "webkitPresentationMode" on type "Element"
if (video.webkitPresentationMode &&
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:72: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "webkitPresentationMode" on type "Element"
video.webkitPresentationMode === 'picture-in-picture') {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:73: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
this.pictureInPictureElement = video;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:83: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
if (value === this.$pictureInPictureElement) return;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:86: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
this.$pictureInPictureElement.removeEventListener(
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:94: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
this.$pictureInPictureElement.addEventListener(
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:110: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "disablePictureInPicture" on type "NamedNodeMap"
this.attributes.disablePictureInPicture ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:110: ERROR - dangerous use of the global this object
this.attributes.disablePictureInPicture ||
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:111: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "webkitSupportsPresentationMode" on type "HTMLVideoElement"
!this.webkitSupportsPresentationMode('picture-in-picture')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:111: ERROR - dangerous use of the global this object
!this.webkitSupportsPresentationMode('picture-in-picture')
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:111: ERROR - Property webkitSupportsPresentationMode never defined on HTMLVideoElement
!this.webkitSupportsPresentationMode('picture-in-picture')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:117: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "webkitSetPresentationMode" on type "HTMLVideoElement"
this.webkitSetPresentationMode('picture-in-picture');
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:117: ERROR - dangerous use of the global this object
this.webkitSetPresentationMode('picture-in-picture');
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:117: ERROR - Property webkitSetPresentationMode never defined on HTMLVideoElement
this.webkitSetPresentationMode('picture-in-picture');
^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:125: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "webkitSetPresentationMode" on type "Element"
document.pictureInPictureElement.webkitSetPresentationMode('inline');
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:125: ERROR - Property webkitSetPresentationMode never defined on Element
document.pictureInPictureElement.webkitSetPresentationMode('inline');
^^^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:128: ERROR - Violation: Throwing non-Error types or Error itself is not allowed; throw shaka.util.Error instead.
throw new DOMException(
^^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:128: ERROR - Function DOMException: called with 2 argument(s). Function requires at least 0 argument(s) and no more than 0 argument(s).
throw new DOMException(
^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:146: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
if (this.webkitPresentationMode === 'picture-in-picture') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:146: ERROR - dangerous use of the global this object
if (this.webkitPresentationMode === 'picture-in-picture') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:146: ERROR - Property webkitPresentationMode never defined on this
if (this.webkitPresentationMode === 'picture-in-picture') {
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:151: ERROR - dangerous use of the global this object
this.addEventListener('webkitpresentationmodechanged', webkitListener);
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:154: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillEnter" on type "HTMLVideoElement"
if (this.$pipPolyfillEnter) {
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:154: ERROR - dangerous use of the global this object
if (this.$pipPolyfillEnter) {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:155: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillEnter" on type "HTMLVideoElement"
this.$pipPolyfillEnter[callback] = webkitListener;
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:155: ERROR - dangerous use of the global this object
this.$pipPolyfillEnter[callback] = webkitListener;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:157: ERROR - dangerous use of the global this object
this.$pipPolyfillEnter = {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:164: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
if (this.webkitPresentationMode === 'inline') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:164: ERROR - dangerous use of the global this object
if (this.webkitPresentationMode === 'inline') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:164: ERROR - Property webkitPresentationMode never defined on this
if (this.webkitPresentationMode === 'inline') {
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:165: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
if (this.$pipPreviousPresentationMode === 'picture-in-picture') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:165: ERROR - dangerous use of the global this object
if (this.$pipPreviousPresentationMode === 'picture-in-picture') {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:170: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
document.pictureInPictureElement = this;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:172: ERROR - dangerous use of the global this object
this.$pipPreviousPresentationMode = this.webkitPresentationMode;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:172: ERROR - dangerous use of the global this object
this.$pipPreviousPresentationMode = this.webkitPresentationMode;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:172: ERROR - Property webkitPresentationMode never defined on this
this.$pipPreviousPresentationMode = this.webkitPresentationMode;
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:175: ERROR - dangerous use of the global this object
this.addEventListener('webkitpresentationmodechanged', webkitListener);
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:176: ERROR - dangerous use of the global this object
this.$pipPreviousPresentationMode = this.webkitPresentationMode;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:176: ERROR - dangerous use of the global this object
this.$pipPreviousPresentationMode = this.webkitPresentationMode;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:176: ERROR - Property webkitPresentationMode never defined on HTMLVideoElement
this.$pipPreviousPresentationMode = this.webkitPresentationMode;
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:179: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillLeave" on type "HTMLVideoElement"
if (this.$pipPolyfillLeave) {
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:179: ERROR - dangerous use of the global this object
if (this.$pipPolyfillLeave) {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:180: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillLeave" on type "HTMLVideoElement"
this.$pipPolyfillLeave[callback] = webkitListener;
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:180: ERROR - dangerous use of the global this object
this.$pipPolyfillLeave[callback] = webkitListener;
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:182: ERROR - dangerous use of the global this object
this.$pipPolyfillLeave = {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:196: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillEnter" on type "HTMLVideoElement"
if (name === 'enterpictureinpicture' && this.$pipPolyfillEnter) {
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:196: ERROR - dangerous use of the global this object
if (name === 'enterpictureinpicture' && this.$pipPolyfillEnter) {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:197: ERROR - dangerous use of the global this object
this.removeEventListener(
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:199: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillEnter" on type "HTMLVideoElement"
this.$pipPolyfillEnter[callback],
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:199: ERROR - dangerous use of the global this object
this.$pipPolyfillEnter[callback],
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:201: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillEnter" on type "HTMLVideoElement"
delete this.$pipPolyfillEnter[callback];
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:201: ERROR - dangerous use of the global this object
delete this.$pipPolyfillEnter[callback];
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:202: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillLeave" on type "HTMLVideoElement"
} else if (name === 'leavepictureinpicture' && this.$pipPolyfillLeave) {
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:202: ERROR - dangerous use of the global this object
} else if (name === 'leavepictureinpicture' && this.$pipPolyfillLeave) {
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:203: ERROR - dangerous use of the global this object
this.removeEventListener(
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:205: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillLeave" on type "HTMLVideoElement"
this.$pipPolyfillLeave[callback],
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:205: ERROR - dangerous use of the global this object
this.$pipPolyfillLeave[callback],
^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:207: ERROR - Violation: Using properties of unknown types is not allowed; add an explicit type to the variable.
The property "$pipPolyfillLeave" on type "HTMLVideoElement"
delete this.$pipPolyfillLeave[callback];
^^^^^^^^^^^^^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:207: ERROR - dangerous use of the global this object
delete this.$pipPolyfillLeave[callback];
^^^^

63 error(s), 0 warning(s), 91.4% typed
[ERROR] Build failed

@joeyparrish

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

There seems to be no license information in the repository you're pulling from, and much of the code looks verbatim. So I'm not sure that I can accept this without a clear license from the author.

But it should be fairly easy to implement from scratch with a link to the Apple docs on this functionality.

The compiler issue is caused by the lack of annotations on the methods you're patching over. For example, @this is used to tell the compiler what the type of this will be when the function is called. You'll find examples of this in our other polyfills.

@avelad

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

Ok, tomorrow I'll rewrite the polyfill from scratch. I hope it can be introduced in version 2.5

@avelad

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2019

I have updated the implementation. But I have 4 error that i can not fix...

[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Generating Closure dependencies...
[INFO] Linting JavaScript...
[INFO] Linting HTML...
[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Checking that the build files are complete...
[INFO] Checking the tests for type errors...
[WARNING] No changes detected, skipping. Use --force to override.
/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:123: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
          if (this.webkitPresentationMode === 'picture-in-picture') {
              ^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:142: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
          if (this.webkitPresentationMode === 'inline') {
              ^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:143: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
            if (this.polyfillPreviousPresentationMode ===
                ^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:149: ERROR - Violation: Failed to infer type of "this"; please be explicit or use bind!
            document.pictureInPictureElement = this;
                                               ^^^^

4 error(s), 0 warning(s), 91.4% typed
[ERROR] Build failed

Can you help me to correct it?

@joeyparrish

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

Can you help me to correct it?

Absolutely! I'd be happy to help. I'll leave specific comments on the changes. Thank you for working on this!

Show resolved Hide resolved externs/pictureinpicture.js Outdated
Show resolved Hide resolved externs/pictureinpicture.js Outdated
Show resolved Hide resolved externs/pictureinpicture.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js
@avelad
Copy link
Contributor Author

left a comment

I updated the code, but i have 6 errors...

[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Generating Closure dependencies...
[INFO] Linting JavaScript...
[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Linting HTML...
[WARNING] No changes detected, skipping. Use --force to override.
[INFO] Checking that the build files are complete...
[INFO] Checking the tests for type errors...
[WARNING] No changes detected, skipping. Use --force to override.
/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:109: ERROR - actual parameter 1 of shaka.polyfill.PiP.getProxyEnterPiPEvent_ does not match formal parameter
found   : (EventListener|function(Event): (boolean|undefined)|null)
required: (EventListener|null)
          shaka.polyfill.PiP.getProxyEnterPiPEvent_(listener);
                                                    ^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:114: ERROR - actual parameter 1 of shaka.polyfill.PiP.getProxyLeavePiPEvent_ does not match formal parameter
found   : (EventListener|function(Event): (boolean|undefined)|null)
required: (EventListener|null)
          shaka.polyfill.PiP.getProxyLeavePiPEvent_(listener);
                                                    ^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:130: ERROR - actual parameter 1 of shaka.polyfill.PiP.getProxyEnterPiPEvent_ does not match formal parameter
found   : (EventListener|function(Event): (boolean|undefined)|null)
required: (EventListener|null)
          shaka.polyfill.PiP.getProxyEnterPiPEvent_(listener);
                                                    ^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:135: ERROR - actual parameter 1 of shaka.polyfill.PiP.getProxyLeavePiPEvent_ does not match formal parameter
found   : (EventListener|function(Event): (boolean|undefined)|null)
required: (EventListener|null)
          shaka.polyfill.PiP.getProxyLeavePiPEvent_(listener);
                                                    ^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:209: ERROR - EventListener expressions are not callable
      listener();
      ^^^^^^^^^^

/home/avelad/workspace/pull-request/shaka-player/lib/polyfill/pip.js:225: ERROR - EventListener expressions are not callable
      listener();
      ^^^^^^^^^^

6 error(s), 0 warning(s), 91.4% typed
[ERROR] Build failed

@joeyparrish , Do you have any idea how to solve it? I have tried many things but none has worked

The functionality works without problem in Safari.

Show resolved Hide resolved lib/polyfill/pip.js
Show resolved Hide resolved lib/polyfill/pip.js Outdated
/**
* polyfill document.pictureInPictureEnabled
*/
document.pictureInPictureEnabled = true;

This comment has been minimized.

Copy link
@avelad

avelad May 1, 2019

Author Contributor

I have tried and the API it in both MSE and src = so I have not seen any reason for this to not be true.

Show resolved Hide resolved lib/polyfill/pip.js Outdated
/**
* polyfill document.pictureInPictureEnabled
*/
document.pictureInPictureEnabled = true;

This comment has been minimized.

Copy link
@joeyparrish

joeyparrish May 1, 2019

Member

Okay, sounds good.

Show resolved Hide resolved lib/polyfill/pip.js Outdated
Show resolved Hide resolved lib/polyfill/pip.js Outdated
/**
* Get the proxy webkitpresentationmodechanged event.
*
* @param {EventListener} listener

This comment has been minimized.

Copy link
@joeyparrish

joeyparrish May 1, 2019

Member

I think the compiler error is because addEventListener has a more complex signature that you would think. It accepts more than just EventListener, so the argument passed here could have other types, as well.

Could you listen to the webkit events all the time and dispatch equivalent standard events when they are seen? Like we do in the fullscreen polyfill?

This comment has been minimized.

Copy link
@avelad

avelad May 1, 2019

Author Contributor

Is not the same case than fullscreen. In fullscreen polyfill the event is trigger over the document. In this case, the event is trigger over the video element. I'll try to find another solution.

This comment has been minimized.

Copy link
@joeyparrish

joeyparrish May 1, 2019

Member

If the only remaining issue is the compiler error on the parameter type here, you could just extend it.

This comment has been minimized.

Copy link
@joeyparrish

joeyparrish May 1, 2019

Member

Try this:

@param {EventListener|function(Event): (boolean|undefined)} listener

This comment has been minimized.

Copy link
@avelad

avelad May 1, 2019

Author Contributor

ERROR - actual parameter 1 of shaka.polyfill.PiP.getProxyLeavePiPEvent_ does not match formal parameter
found : (EventListener|function(Event): (boolean|undefined)|null)
required: (EventListener|function((Event|null)): (boolean|undefined)|null)

if (videoElement.webkitPresentationMode === 'picture-in-picture') {
// keep track of the pipElement
document.pictureInPictureElement = videoElement;
listener();

This comment has been minimized.

Copy link
@joeyparrish

joeyparrish May 1, 2019

Member

Here you'll need to call listener(event) with the event, since the compiler expects an argument. But if the listener is looking at event.type for some reason, it might get confused by seeing a type it didn't listen for.

I think this is another good argument for redispatching events.

@avelad

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

I updated the implementacion without compiler issues and working in safari.

/** @type {HTMLMediaElement} */
let polyfillPictureInPictureElement = null;

let polyfillEnterpictureinpicture = null;

This comment has been minimized.

Copy link
@joeyparrish

joeyparrish May 1, 2019

Member

I think we may be avoiding compiler errors by leaving out types. I appreciate all the work you've done on this, though, and I know the compiler can be a pain sometimes. I think perhaps we should merge this PR as-is and I will work on some follow-up changes. Would that be alright?

This comment has been minimized.

Copy link
@avelad

avelad May 2, 2019

Author Contributor

For me it's ok!
Thanks for your revision, I really appreciate it and it help to another PR in coming.

@shaka-bot

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

All tests passed!

@avelad

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

@joeyparrish , any news about this merge?

@joeyparrish

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Sorry, we've been busy wrapping up other things. I'm merging now, and I'll work on a that little bit of cleanup after that. Thanks for your contribution!

@joeyparrish joeyparrish merged commit caa34ca into google:master May 3, 2019

2 checks passed

cla/google All necessary CLAs are signed
shaka-build-bot All tests passed
@joeyparrish

This comment has been minimized.

Copy link
Member

commented May 3, 2019

I've experimented a bit and found a much simpler way to manage the events. It turns out that you can listen on document in the capturing phase to avoid having to shim the listeners on the video elements.

shaka-bot pushed a commit that referenced this pull request May 3, 2019

Rename PiP polyfill to PiPWebkit
Follow-up to #1902

Change-Id: I760d0006c4cbd7c3aad2ce96801259b578269195

shaka-bot pushed a commit that referenced this pull request May 7, 2019

Simplify Webkit PiP polyfill
By listening to events on document in the capturing phase, we can
avoid complex event shims on HTMLVideoElement.  By monitoring
global PiP state on the document, we can get rid of the getter and
setter for the document's PiP element, as well.

Follow-up on PR #1902

Change-Id: Ib5f5d0c7c735124a438901c5bb15e034ab10b996
@joeyparrish

This comment has been minimized.

Copy link
Member

commented May 7, 2019

@avelad, I've just merged the last of my changes to your polyfill. Thanks again for working on this! It's great to see Safari and iOS getting so many useful features beyond merely "src= playback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.