-
Notifications
You must be signed in to change notification settings - Fork 44
Overhaul exit Picture-in-Picture algorithm #260
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
base: main
Are you sure you want to change the base?
Changes from all commits
7ed1fed
4fb83af
89b43f2
a63d659
39a9130
846790f
bf7bba9
00918f8
5330f78
81e4c56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -221,30 +221,44 @@ Picture-in-Picture window. | |||||||||||||
|
|
||||||||||||||
| ## Exit Picture-in-Picture ## {#exit-pip} | ||||||||||||||
|
|
||||||||||||||
| When the <dfn>exit Picture-in-Picture algorithm</dfn> is invoked, | ||||||||||||||
| the user agent MUST run the following steps: | ||||||||||||||
|
|
||||||||||||||
| 1. If {{pictureInPictureElement}} is `null`, throw a {{InvalidStateError}} and | ||||||||||||||
| abort these steps. | ||||||||||||||
| 2. Run the <a>close window algorithm</a> with the <a>Picture-in-Picture | ||||||||||||||
| window</a> associated with {{pictureInPictureElement}}. | ||||||||||||||
| 3. <a>Queue a task</a> to <a>fire an event</a> named | ||||||||||||||
| {{leavepictureinpicture}} using {{PictureInPictureEvent}} at the | ||||||||||||||
| |video| with its {{bubbles}} attribute initialized to `true` and its | ||||||||||||||
| {{PictureInPictureEvent/pictureInPictureWindow}} attribute initialized to | ||||||||||||||
| <a>Picture-in-Picture window</a> associated with {{pictureInPictureElement}}. | ||||||||||||||
| 4. Unset {{pictureInPictureElement}}. | ||||||||||||||
| 5. Remove one <a>item</a> matching <a>relevant settings object</a>'s <a>origin</a> from | ||||||||||||||
| <a>initiators of active Picture-in-Picture sessions</a>. | ||||||||||||||
| When the <dfn>exit Picture-in-Picture algorithm</dfn> is invoked given |doc|, and nullable | ||||||||||||||
| {{Promise}} |p|, run these steps: | ||||||||||||||
|
|
||||||||||||||
| 1. [=Assert=] that these steps are running on the [=picture-in-picture parallel queue=]. | ||||||||||||||
| 2. Let |global| be |doc|'s [=relevant global object=]. | ||||||||||||||
| 3. Run the <a>close window algorithm</a> with the <a>Picture-in-Picture window</a> associated with | ||||||||||||||
| |doc|'s [=Picture-in-Picture element=]. | ||||||||||||||
| 4. [=Queue a global task=] on the [=media element event task source=] given |global|, to perform the | ||||||||||||||
| following steps: | ||||||||||||||
| 1. If |doc| is not [=fully active=] or |doc|'s [=Picture-in-Picture element=] is `null`: | ||||||||||||||
| 1. If |p| is not `null`, [=/resolve=] |p| with undefined. | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I naively thought we should reject instead of resolve promise. Why not in this case?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree... this should reject with an InvalidStateError.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, these should happen both before [=Queue a global task=] and recheck after [=Queue a global task=]. See https://www.w3.org/TR/design-principles/#support-non-fully-active
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Left a comment in the main thread, but I think there's an argument for that we should not be doing this check at all. |
||||||||||||||
| 2. Return. | ||||||||||||||
| 2. Let |element| be |doc|'s [=Picture-in-Picture element=]. | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can element be null? |
||||||||||||||
| 3. Set |doc|'s [=Picture-in-Picture element=] to `null`. | ||||||||||||||
| 4. <a>Fire an event</a> named {{leavepictureinpicture}} using {{PictureInPictureEvent}} at the | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [=Queue a task=] to perform the follow steps? |
||||||||||||||
| |element| with its {{bubbles}} attribute initialized to `true` and its | ||||||||||||||
| {{PictureInPictureEvent/pictureInPictureWindow}} attribute initialized to | ||||||||||||||
| <a>Picture-in-Picture window</a> associated with |element|. | ||||||||||||||
| 5. Remove one <a>item</a> matching <a>relevant settings object</a>'s <a>origin</a> from | ||||||||||||||
| <a>initiators of active Picture-in-Picture sessions</a>. | ||||||||||||||
| 6. If |p| is not `null`, [=/resolve=] |p| with undefined. | ||||||||||||||
|
|
||||||||||||||
| It is NOT RECOMMENDED that the video playback state changes when the <a>exit | ||||||||||||||
| Picture-in-Picture algorithm</a> is invoked. The website SHOULD be in control | ||||||||||||||
| of the experience if it is website initiated. However, the user agent MAY expose | ||||||||||||||
| Picture-in-Picture window controls that change video playback state (e.g., | ||||||||||||||
| pause). | ||||||||||||||
|
|
||||||||||||||
| As one of the <a>unloading document cleanup steps</a>, run the <a>exit | ||||||||||||||
| Picture-in-Picture algorithm</a>. | ||||||||||||||
| ### Unloading steps ### {#unloading-steps} | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talking to @chrisn, would be great to just move these over to HTML. |
||||||||||||||
| As one of the <a>unloading document cleanup steps</a> given |doc| run the following steps: | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Exported so we can call this from HTML
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See whatwg/html#12689 for the corresponding HTML change. |
||||||||||||||
| 1. If |doc|'s [=Picture-in-Picture element=] is `null`, return. | ||||||||||||||
| 2. Let |window| be the <a>Picture-in-Picture window</a> associated with |doc|'s | ||||||||||||||
| [=Picture-in-Picture element=]. | ||||||||||||||
| 3. Set |doc|'s [=Picture-in-Picture element=] to `null`. | ||||||||||||||
| 4. Remove one <a>item</a> matching <a>relevant settings object</a>'s <a>origin</a> from | ||||||||||||||
| <a>initiators of active Picture-in-Picture sessions</a>. | ||||||||||||||
| 5. [=Enqueue the following steps=] to |doc|'s <a>picture-in-picture parallel queue</a>: | ||||||||||||||
| 1. Run the <a>close window algorithm</a> with |window|. | ||||||||||||||
|
|
||||||||||||||
| ## Disable Picture-in-Picture ## {#disable-pip} | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -271,8 +285,12 @@ the user agent MAY run these steps: | |||||||||||||
|
|
||||||||||||||
| ## Interaction with Fullscreen ## {#fullscreen} | ||||||||||||||
|
|
||||||||||||||
| It is RECOMMENDED to run the <a>exit Picture-in-Picture algorithm</a> when the | ||||||||||||||
| {{pictureInPictureElement}} <a>fullscreen flag</a> is set. | ||||||||||||||
| It is RECOMMENDED that when a [=Picture-in-Picture element=]'s <a>fullscreen flag</a> is set, the | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we talked about this, so apologies... why is the RECOMMENDED instead of MUST? Can we add a note here explaining why it's not a MUST requirement?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to invoke this from step 3 in https://fullscreen.spec.whatwg.org/#fullscreen-an-element
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See also #191. But let's work on that in a follow up PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, yeah... it's basically transitioning in/out of fullscreen otherwise PiP, |
||||||||||||||
| user agent runs these steps: | ||||||||||||||
|
|
||||||||||||||
| 1. Let |doc| be the [=Picture-in-Picture element=]'s [=node document=]. | ||||||||||||||
| 2. [=Enqueue the following steps=] to |doc|'s <a>picture-in-picture parallel queue</a>: | ||||||||||||||
| 1. Run the <a>exit Picture-in-Picture algorithm</a> given |doc| and `null`. | ||||||||||||||
|
|
||||||||||||||
| ## Interaction with Page Visibility ## {#page-visibility} | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -340,18 +358,19 @@ The {{requestPictureInPicture()}} method steps <dfn export>request Picture-in-Pi | |||||||||||||
| [=/reject=] |p| with {{InvalidStateError}} {{DOMException}}. | ||||||||||||||
| 2. Abort these steps. | ||||||||||||||
| 4. Let |pipWindow| be a new instance of {{PictureInPictureWindow}} that represents [=this=]'s associated [=Picture-in-Picture window=]. | ||||||||||||||
| 5. [=Queue a global task=] on the [=media element event task source=] given |global|, to perform | ||||||||||||||
| 5. If |doc|'s [=Picture-in-Picture element=] is not `null`, run the [=exit Picture-in-Picture | ||||||||||||||
| algorithm=] given |doc| and `null`. | ||||||||||||||
| 6. [=Queue a global task=] on the [=media element event task source=] given |global|, to perform | ||||||||||||||
| the following steps: | ||||||||||||||
| 1. If {{pictureInPictureElement}} is not `null`, run the [=exit Picture-in-Picture algorithm=]. | ||||||||||||||
| 2. Set |doc|'s [=Picture-in-Picture element=] to [=this=]. | ||||||||||||||
| 3. [=list/Append=] [=relevant settings object=]'s [=origin=] to [=initiators of active | ||||||||||||||
| 1. Set |doc|'s [=Picture-in-Picture element=] to [=this=]. | ||||||||||||||
| 2. [=list/Append=] [=relevant settings object=]'s [=origin=] to [=initiators of active | ||||||||||||||
| Picture-in-Picture sessions=]. | ||||||||||||||
| 4. If [=this=] is [=fullscreenElement=], [=exit fullscreen=]. | ||||||||||||||
| 5. [=Fire an event=] named {{enterpictureinpicture}} using {{PictureInPictureEvent}} at | ||||||||||||||
| 3. If [=this=] is [=fullscreenElement=], then [=exit fullscreen=]. | ||||||||||||||
| 4. [=Fire an event=] named {{enterpictureinpicture}} using {{PictureInPictureEvent}} at | ||||||||||||||
| [=this=] with its {{bubbles}} attribute initialized to `true` and its | ||||||||||||||
| {{PictureInPictureEvent/pictureInPictureWindow}} attribute initialized to | ||||||||||||||
| [=Picture-in-Picture window=]. | ||||||||||||||
| 6. [=/Resolve=] |p| with |pipWindow|. | ||||||||||||||
| 5. [=/Resolve=] |p| with |pipWindow|. | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| ## Extensions to <code>Document</code> ## {#document-extensions} | ||||||||||||||
|
|
@@ -372,14 +391,18 @@ The {{pictureInPictureEnabled}} attribute's getter must return `true` if | |||||||||||||
| <dfn>Picture-in-Picture support</dfn> is `false` if there's a user preference | ||||||||||||||
| that disables it or a platform limitation. It is `true` otherwise. | ||||||||||||||
|
|
||||||||||||||
| The {{exitPictureInPicture()}} method, when invoked, MUST | ||||||||||||||
| return <a>a new promise</a> |promise| and run the following steps <a>in | ||||||||||||||
| parallel</a>: | ||||||||||||||
|
|
||||||||||||||
| 1. Run the <a>exit Picture-in-Picture algorithm</a>. | ||||||||||||||
| 2. If the previous step threw an exception, reject |promise| with that | ||||||||||||||
| exception and abort these steps. | ||||||||||||||
| 3. [=/Resolve=] |promise|. | ||||||||||||||
| The {{exitPictureInPicture()}} method, when invoked, runs the following steps: | ||||||||||||||
|
|
||||||||||||||
| 1. If [=this=]'s [=Picture-in-Picture element=] is `null` or [=this=] is not [=fully active=], | ||||||||||||||
| return [=a promise rejected with=] {{InvalidStateError}} {{DOMException}}. | ||||||||||||||
|
Comment on lines
+396
to
+397
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit:
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also have web-platform tests along this PR to help figuring out what needs to be fixed in different implementations?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually wrote a postMessage web platform tests specifically for this "not fully active" scenario, and it involves actually testing that something "did not happen" - and we wouldn't be able to test this here, I think. This promise will never get resolve or rejected, I believe. It should just be doing the null check. |
||||||||||||||
| 2. Let |p| be [=a new promise=] created in [=this=]'s [=relevant realm=]. | ||||||||||||||
| 3. Return |p|, and [=enqueue the following steps=] to [=this=]'s [=picture-in-picture parallel | ||||||||||||||
| queue=]: | ||||||||||||||
| 1. If [=this=]'s [=Picture-in-Picture element=] is `null`: | ||||||||||||||
| 1. [=Queue a global task=] on the [=media element event task source=] given [=this=]'s | ||||||||||||||
| [=relevant global object=] to [=/resolve=] |p|. | ||||||||||||||
| 2. Return. | ||||||||||||||
| 2. Run the <a>exit Picture-in-Picture algorithm</a> given [=this=] and |p|. | ||||||||||||||
|
|
||||||||||||||
| ## Extension to <code>DocumentOrShadowRoot</code> ## {#documentorshadowroot-extension} | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.