Add EnterPictureInPictureReason #362
Conversation
index.bs
Outdated
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">useraction</dfn>: the user has taken an explicit action to enter picture-in-picture (e.g. clicking a picture-in-picture button in the user agent UI) | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">contentoccluded</dfn>: the user agent is requesting picture-in-picture because the content has become occluded |
There was a problem hiding this comment.
Is this precise enough?
I am guessing this covers window minimisation and full web page occlusion (tab switching for instance), which would be related to DOM visibilityState.
What about partial occlusion by other OS windows?
What about occlusion of the video element by content from the same page, or if user is scrolling the web page? Is the term content corresponding to video element or web page?
There was a problem hiding this comment.
Chrome currently doesn't trigger this for partial occlusion by other windows, but I suppose we could eventually but I don't see an issue still referring to this as content occlusion.
It's corresponding to the web page. Chrome has no plans for firing an enterpictureinpicture event when a webpage is scrolled or another element covers a video since that is something the website should decide how to handle.
Is there a better way to describe this?
There was a problem hiding this comment.
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">contentoccluded</dfn>: the user agent is requesting picture-in-picture because the content has become occluded | |
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">contentoccluded</dfn>: the user agent is requesting picture-in-picture because the page has become occluded. This can happen in various cases like tab switching or tab minimisation. |
index.bs
Outdated
| {{enterPictureInPictureReason}} can have one of the following values: | ||
| <ul> | ||
| <li> | ||
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">unspecified</dfn>: the reason for entering picture-in-picture is unspecified |
There was a problem hiding this comment.
What is the use case for that value? Will Chrome implementation make use of that value?
There was a problem hiding this comment.
Chrome has no plans to use this value. In my mind it was there so a UA could trigger enterpip but not specify a value, but I suppose there is no current use case for that, so we could remove it
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new EnterPictureInPictureReason enum to provide context about why the picture-in-picture action was triggered, enhancing the MediaSession API's picture-in-picture functionality.
- Introduces
MediaSessionEnterPictureInPictureReasonenum with three values: "unspecified", "useraction", and "contentoccluded" - Adds
enterPictureInPictureReasonfield toMediaSessionActionDetailsdictionary for theenterpictureinpictureaction - Documents the requirement that this field must be provided when the action is
enterpictureinpicture
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| double seekTime; | ||
| boolean fastSeek; | ||
| boolean isActivating; | ||
| MediaSessionEnterPictureInPictureReason enterPictureInPictureReason; |
There was a problem hiding this comment.
The field enterPictureInPictureReason should be marked as optional since it's only relevant for the enterpictureinpicture action. Consider adding a question mark to make it MediaSessionEnterPictureInPictureReason? enterPictureInPictureReason; to clearly indicate it's optional for other actions.
| MediaSessionEnterPictureInPictureReason enterPictureInPictureReason; | |
| MediaSessionEnterPictureInPictureReason? enterPictureInPictureReason; |
There was a problem hiding this comment.
This is the opposite of the suggestion I received from another reviewer, so not sure what to make of it
index.bs
Outdated
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">useraction</dfn>: the user has taken an explicit action to enter picture-in-picture (e.g. clicking a picture-in-picture button in the user agent UI) | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">contentoccluded</dfn>: the user agent is requesting picture-in-picture because the content has become occluded |
There was a problem hiding this comment.
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">contentoccluded</dfn>: the user agent is requesting picture-in-picture because the content has become occluded | |
| <dfn enum-value for="MediaSessionEnterPictureInPictureReason">contentoccluded</dfn>: the user agent is requesting picture-in-picture because the page has become occluded. This can happen in various cases like tab switching or tab minimisation. |
SHA: dd37d72 Reason: push, by steimelchrome Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This CL adds a reason to be sent alongside `enterpictureinpicture` Media Session actions to indicate the source of the picture-in-picture request. Spec change: w3c/mediasession#362 Chromestatus: https://chromestatus.com/feature/6415506970116096 Bug: 446738067 Change-Id: I1fcce17ed5ecba25a369ca5cc7a03440f52f2cc7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6838265 Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Tommy Steimel <steimel@chromium.org> Cr-Commit-Position: refs/heads/main@{#1520260}
This adds
EnterPictureInPictureReasonto theMediaSessionActionDetailsfor theenterpictureinpictureaction, based on the discussions surrounding #358Preview | Diff