refactor(packages)!: normalize fullscreen and picture-in-picture host APIs#1468
refactor(packages)!: normalize fullscreen and picture-in-picture host APIs#1468
Conversation
… APIs BREAKING CHANGE: PlayerTarget.media is now typed as Video. Presentation helpers (presentation/fullscreen.ts, presentation/pip.ts) no longer accept WebKit-specific overloads — call media.requestFullscreen() etc. directly. - Add isFullscreen and isPictureInPicture getters to HTMLVideoElementHost - Centralize WebKit event normalization in the host: webkitfullscreenchange and webkitpresentationmodechanged are mapped to bubbling fullscreenchange and enter/leavepictureinpicture - Prefer webkitEnterFullscreen on iOS Safari (regression fix from a05e8f2) - Resolve requestPictureInPicture/exitPictureInPicture promises when the corresponding event fires (WebKit setPresentationMode is fire-and-forget) - Add toMediaHost helper that wraps raw HTMLVideoElement / HTMLAudioElement so store.attach always sees the full host API - Wire toMediaHost through @videojs/html provider-mixin and @videojs/react create-player - Keep controls visible while in picture-in-picture Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📦 Bundle Size Report🎨 @videojs/html
Presets (7)
Media (8)
Players (3)
Skins (29)
UI Components (25)
Sizes are marginal over the root entry point. ⚛️ @videojs/react
Presets (7)
Media (7)
Skins (26)
UI Components (20)
Sizes are marginal over the root entry point. 🧩 @videojs/core
Entries (10)
🏷️ @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}
Isolates HTMLVideoElementHost and HTMLAudioElementHost from the main @videojs/core/dom barrel so consumers that don't wrap raw <video> / <audio> elements no longer pay the cost of shipping the host classes. Only @videojs/html and @videojs/react import from the new subpath. Made-with: Cursor
| this.#wasPictureInPicture = isPip; | ||
| this.dispatchEvent(new Event(isPip ? 'enterpictureinpicture' : 'leavepictureinpicture')); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Duplicate PiP events from WebKit normalization and base forwarding
Medium Severity
The host's #handlePresentationChange synthesizes enterpictureinpicture/leavepictureinpicture events from WebKit's webkitpresentationmodechanged. Simultaneously, the base class addEventListener override auto-registers a #forwardEvent listener on the video target for these same event types whenever a feature subscribes (e.g., listen(media, 'enterpictureinpicture', ...)). On Safari, where user-initiated PiP (via browser controls) can fire both webkitpresentationmodechanged and native enterpictureinpicture, listeners on the host receive duplicate events. The #wasPictureInPicture guard only deduplicates synthetic events, not forwarded native ones.
Reviewed by Cursor Bugbot for commit 0fb7c58. Configure here.
| const target = (media as EventTarget & { target?: unknown }).target; | ||
| return target instanceof HTMLMediaElement ? target : media; | ||
| } | ||
|
|
There was a problem hiding this comment.
Safari PWA PiP availability exclusion silently dropped
Low Severity
isPictureInPictureEnabled previously returned false for Safari Progressive Web Apps (standalone display mode) because the standard PiP API doesn't work reliably in that context. The new code removes the isPWA check entirely: on Safari, it now always falls through to the webkitSetPresentationMode capability check, which returns true. If WebKit's presentation mode API also doesn't work in Safari PWAs, the player would show a non-functional PiP button.
Reviewed by Cursor Bugbot for commit 0fb7c58. Configure here.
Update hls, dash, simple-hls, and native-hls to import HTMLVideoElementHost from '../host' instead of '../video-host', so the host classes have a single canonical import point. tsdown's unbundle mode resolves the re-export so consumers don't pay any runtime cost. Made-with: Cursor
|
this doesn't work very well. the wrench is the native video element in html, if we convert that to our media host API with normalizations it has to be done in the provider mixin and that increases bundle size for others. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca36017. Configure here.
| * @label Media | ||
| * @param media - A {@link Media}-shaped value (e.g. an existing host); returned unchanged. | ||
| */ | ||
| export function toMediaHost(media: Media): ToMediaHostResult<Video>; |
There was a problem hiding this comment.
Type overload unsafely widens Media to Video
Medium Severity
The toMediaHost(media: Media): ToMediaHostResult<Video> overload promises the result contains a Video, but at runtime non-element inputs (like HTMLAudioElementHost, which implements Audio but not Video) pass through unchanged via the fallback path. Downstream code (e.g., PlayerTarget.media: Video) then accesses isFullscreen, isPictureInPicture, exitPictureInPicture() etc. on an object that lacks them, leading to undefined property access at runtime.
Reviewed by Cursor Bugbot for commit ca36017. Configure here.
| return; | ||
| } | ||
|
|
||
| throw new DOMException('Picture-in-Picture not supported', 'NotSupportedError'); |
There was a problem hiding this comment.
WebKit PiP promises hang if mode change fails
Low Severity
requestPictureInPicture and exitPictureInPicture use onEvent to await enterpictureinpicture / leavepictureinpicture after calling webkitSetPresentationMode. If the mode change fails silently (e.g., system blocks PiP, or the browser is in an unsupported context like Safari PWA), the webkitpresentationmodechanged event never fires, the synthetic event is never dispatched, and the returned promise hangs indefinitely with no timeout or abort mechanism.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ca36017. Configure here.
1 participant
Summary
Normalize the video host's fullscreen and picture-in-picture APIs around
standard methods (
requestFullscreen,exitFullscreen,requestPictureInPicture,exitPictureInPicture) plusisFullscreenandisPictureInPicturegetters. WebKit-specific quirks (webkit*events andmethods) are now centralized in
HTMLVideoElementHostso features andpresentation helpers can stay platform-agnostic.
Changes
HTMLVideoElementHost. WebKitevents (
webkitpresentationmodechanged,webkitfullscreenchange) arenormalized into bubbling
fullscreenchangeandenter/leavepictureinpictureevents.requestPictureInPictureandexitPictureInPicturenow resolve only after the corresponding eventfires (WebKit's
setPresentationModeis fire-and-forget).webkitEnterFullscreenover the standard API on
<video>, where the latter silently fails(originally fixed in a05e8f2).
toMediaHosthelper — New helper that wraps rawHTMLVideoElement/HTMLAudioElementinstances into their host classes. Lives at@videojs/core/dom/media/host(a dedicated subpath) so the host classesstay out of the main
@videojs/core/dombarrel — only@videojs/htmland
@videojs/reactimport from it.picture-in-picture, mirroring the existing paused / remote-playback
behavior.
Breaking changes
PlayerTarget.mediais now typed asVideoinstead ofMedia.presentation/fullscreen.tsandpresentation/pip.tsno longer acceptWebKit-specific overloads — call
media.requestFullscreen()etc.directly.
exitFullscreenno longer takes acontainerargument; it picks thecorrect exit path from
media.isFullscreen.Testing
pnpm typecheckpnpm -F @videojs/core test— 1066 testspnpm -F @videojs/html test— 101 testspnpm -F @videojs/react test— 200 teststoMediaHost, host PiP/fullscreen behavior, WebKit eventnormalization, and PiP-aware controls visibility.
Made with Cursor
Note
Medium Risk
Medium risk due to breaking type/API changes around
PlayerTarget.mediaand fullscreen/PiP handling, plus new wrapping lifecycle that could affect attach/detach behavior across consumers and platforms (notably iOS WebKit).Overview
Normalizes fullscreen and picture-in-picture APIs across the DOM player stack by extending media capability types with
isFullscreen/exitFullscreenandisPictureInPicture/exitPictureInPicture, and moving WebKit-specific behavior intoHTMLVideoElementHost(including event normalization and promise timing).Introduces
toMediaHostunder@videojs/core/dom/media/hostto wrap raw<video>/<audio>elements into host classes and provide arelease()hook;@videojs/htmland@videojs/reactnow wrap media beforestore.attach()and clean up the wrapper on detach.Updates player features to rely on host capabilities (fullscreen/PiP/controls visibility) and trims presentation helpers (
pip.tsnow only exposes enablement;fullscreen.tsdelegates exit to media when element-level fullscreen is active). Extensive tests are added/updated to use host-based media and to cover WebKit fullscreen/PiP edge cases and controls staying visible during PiP.Reviewed by Cursor Bugbot for commit ca36017. Bugbot is set up for automated code reviews on this repo. Configure here.