refactor(core)!: unify fullscreen and pip on media capabilities#1469
Merged
refactor(core)!: unify fullscreen and pip on media capabilities#1469
Conversation
Extends MediaFullscreenCapability and MediaPictureInPictureCapability with isFullscreen/isPictureInPicture getters and exitFullscreen/ exitPictureInPicture methods so the host owns presentation state, and drops the legacy webkitDisplayingFullscreen / webkitEnterFullscreen / webkitExitFullscreen branches in favor of webkitSetPresentationMode. Renames isFullscreenElement -> isFullscreen and isPictureInPictureElement -> isPictureInPicture, and adds :fullscreen pseudo-class detection so fullscreen is recognized when requested on a descendant inside a shadow tree. Made-with: Cursor
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (8)
Players (3)
Skins (29)
UI Components (25)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (7)
Skins (26)
UI Components (20)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (9)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mihar-22
previously approved these changes
Apr 25, 2026
Member
mihar-22
left a comment
mihar-22
left a comment
There was a problem hiding this comment.
Perfect! Thank you for doing this ❤️
Small recommendations and questions below.
| return this.target?.requestPictureInPicture() ?? Promise.reject(); | ||
| get isPictureInPicture(): boolean { | ||
| return ( | ||
| (!!this.target && document.pictureInPictureElement === this.target) || |
Member
There was a problem hiding this comment.
I think this can just be document.pictureInPictureElement === this.target?
Collaborator
Author
There was a problem hiding this comment.
what if they're both null?
- broaden capability return types to Promise<unknown> so adapters can forward native results - guard webkitSetPresentationMode by current presentation mode before exiting - use globalThis.document and remove redundant throws so unsupported paths no-op Made-with: Cursor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c4da2bd. Configure here.
The exitFullscreen path now requires webkitPresentationMode === 'fullscreen'
before calling webkitSetPresentationMode('inline'), so the mock must reflect
that state.
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lift fullscreen and picture-in-picture state onto the media capability interfaces so the host (rather than the presentation utilities) owns "am I in this mode?", and collapse the WebKit branches onto a single
webkitSetPresentationModepath.Changes
MediaFullscreenCapabilitygainsisFullscreen+exitFullscreen;MediaPictureInPictureCapabilitygainsisPictureInPicture+exitPictureInPicture.requestPictureInPictureis nowPromise<void>.HTMLVideoElementHostimplements the new getters/methods, deriving state fromdocument.fullscreenElement/document.pictureInPictureElementandwebkitPresentationMode.webkitDisplayingFullscreen,webkitEnterFullscreen,webkitExitFullscreenfrom the WebKit type and all call sites —webkitSetPresentationMode('fullscreen' | 'inline' | 'picture-in-picture')covers everything.isFullscreenElement→isFullscreenandisPictureInPictureElement→isPictureInPicture; both now consult the capability getters as a final fallback so non-HTMLMediaElementhosts work.:fullscreenpseudo-class so requests issued on a descendant inside a shadow tree (e.g. native controls fullscreening the inner<video>) are still recognized.exitFullscreennow requires amediaargument so the WebKit and capability fallbacks have a target.Why
Presentation utilities previously assumed
mediawas anHTMLMediaElementand reached for vendor APIs directly. Pushing the "is this thing in fullscreen / PiP" decision into the host lets custom media targets (mse-shim, react-native, etc.) participate without the utilities special-casing them, and removes the iOS-vs-everything-else fork insiderequestFullscreen/exitFullscreen.Testing
HTMLVideoElementHostunit tests coverisFullscreen/isPictureInPictureacross DOM and WebKit paths.:fullscreenpath, and the host-driven flow.pnpm -F @videojs/core testMade with Cursor
Note
Medium Risk
Medium risk because fullscreen/PiP detection and enter/exit behavior is refactored across multiple presentation utilities and player features, with new capability requirements and Safari/WebKit path changes that could affect edge-case platform behavior.
Overview
Refactors fullscreen and picture-in-picture to be capability-driven:
MediaFullscreenCapability/MediaPictureInPictureCapabilitynow exposeisFullscreen/isPictureInPictureplusexitFullscreen/exitPictureInPicture, andHTMLVideoElementHostimplements these by checkingdocument.*Elementand WebKitwebkitPresentationMode.Presentation helpers are updated to use the new capabilities as a fallback for non-
HTMLMediaElementmedia, and Safari handling is collapsed ontowebkitSetPresentationMode(removingwebkitEnterFullscreen/webkitExitFullscreen/webkitDisplayingFullscreen). Fullscreen detection is also expanded to use the:fullscreenpseudo-class to handle shadow DOM/descendant fullscreen scenarios.Player store features (
fullscreen,pip,remote-playback) are updated to the renamed helpers (isFullscreen,isPictureInPicture) and to passmediaintoexitFullscreen, with extensive new/updated tests covering host-based media and the new detection paths.Reviewed by Cursor Bugbot for commit d421954. Bugbot is set up for automated code reviews on this repo. Configure here.