From 8138c83558987d98c76d125970c6e43e8a6c5834 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 21 Sep 2026 11:02:01 +0200 Subject: [PATCH] fix(journey): the video player brings its own icons instead of asking cdn.plyr.io Plyr's defaults point at its CDN twice: the icon sprite for the controls is fetched over XHR, and on teardown the element is pointed at a blank clip there to abort the stream. connect-src and media-src both refuse that host, so the clip played but every button in the player was blank, and each close of the lightbox logged a blocked request and an uncaught XHR error. The sprite ships with the bundle now, served from this origin, which Plyr then references directly instead of loading it. plyr's exports field hides the file, so it goes through the same resolver alias the rtl-text plugin already needed. The teardown source is empty: that aborts the stream just the same, and the element is unmounted in the same tick. Refs #2341 --- client/rtlTextAlias.js | 14 ++++++ .../components/Journey/VideoPlayer.test.tsx | 48 +++++++++++++++++++ client/src/components/Journey/VideoPlayer.tsx | 11 +++++ client/vite.config.js | 4 +- client/vitest.config.ts | 4 +- 5 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 client/src/components/Journey/VideoPlayer.test.tsx diff --git a/client/rtlTextAlias.js b/client/rtlTextAlias.js index 3bfaa367a..9709c55b1 100644 --- a/client/rtlTextAlias.js +++ b/client/rtlTextAlias.js @@ -23,3 +23,17 @@ export const rtlTextAlias = { '../dist/mapbox-gl-rtl-text.js', ), }; + +/** + * Same shape for Plyr's icon sprite. The package publishes its JS and CSS entries + * and nothing else, so `plyr/dist/plyr.svg` is refused by the resolver although + * the file is right there. The player has to point at a copy on this origin + * (#2341): its default is the sprite on cdn.plyr.io, which the CSP refuses. + */ +export const plyrSpriteAlias = { + find: /^plyr\/dist\/plyr\.svg/, + replacement: path.join( + path.dirname(createRequire(import.meta.url).resolve('plyr')), + 'plyr.svg', + ), +}; diff --git a/client/src/components/Journey/VideoPlayer.test.tsx b/client/src/components/Journey/VideoPlayer.test.tsx new file mode 100644 index 000000000..738f2db4e --- /dev/null +++ b/client/src/components/Journey/VideoPlayer.test.tsx @@ -0,0 +1,48 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +const plyrCtor = vi.fn() +vi.mock('plyr', () => ({ + default: class { + constructor(el: HTMLElement, options: Record) { + plyrCtor(el, options) + } + destroy = vi.fn() + }, +})) +vi.mock('plyr/dist/plyr.css', () => ({})) + +import { render } from '../../../tests/helpers/render' +import VideoPlayer from './VideoPlayer' + +describe('VideoPlayer', () => { + beforeEach(() => { plyrCtor.mockClear() }) + + it('hands Plyr the sprite from this origin, not the one on cdn.plyr.io (#2341)', () => { + // connect-src refuses cdn.plyr.io, so with the default the player worked and + // every button in it was blank. The sprite ships with the bundle instead. + render() + + expect(plyrCtor).toHaveBeenCalledTimes(1) + const [, options] = plyrCtor.mock.calls[0] + // A hashed asset URL in the bundle, the raw file (with its query) under vitest. + expect(String(options.iconUrl)).toMatch(/plyr[^/]*\.svg(\?|$)/) + expect(String(options.iconUrl)).not.toContain('cdn.plyr.io') + }) + + it('resets the element to an empty source on teardown rather than a clip on the CDN', () => { + // media-src refuses cdn.plyr.io too; an empty source aborts the stream just the + // same, and the element is unmounted in the same tick. + render() + + const [, options] = plyrCtor.mock.calls[0] + expect(options.blankVideo).toBe('') + }) + + it('mounts the player on the video element it renders', () => { + const { container } = render() + + const [el] = plyrCtor.mock.calls[0] + expect(el).toBe(container.querySelector('video')) + expect(container.querySelector('video')).toHaveAttribute('poster', '/api/photos/7/thumbnail') + }) +}) diff --git a/client/src/components/Journey/VideoPlayer.tsx b/client/src/components/Journey/VideoPlayer.tsx index f5dc79e06..7ac767301 100644 --- a/client/src/components/Journey/VideoPlayer.tsx +++ b/client/src/components/Journey/VideoPlayer.tsx @@ -1,6 +1,11 @@ import React, { useEffect, useRef } from 'react' import Plyr from 'plyr' import 'plyr/dist/plyr.css' +// The control icons, served from this origin. Plyr's default points at cdn.plyr.io, +// which connect-src refuses: the player worked but every button in it was blank. +// A file, not a data URL, because Plyr tells same-origin from cross-origin by the +// host and would fetch a data URL over XHR, which connect-src refuses as well. +import plyrSprite from 'plyr/dist/plyr.svg?no-inline' /** * Video player for gallery/lightbox playback (#823), built on Plyr over a native @@ -40,6 +45,12 @@ export default function VideoPlayer({ autoplay: autoPlay, clickToPlay: true, hideControls: false, + iconUrl: plyrSprite, + // On teardown Plyr points the element at a blank clip to abort the stream, + // by default one on cdn.plyr.io, which media-src refuses. An empty source + // aborts the stream just the same, and the element is unmounted in the same + // tick, so the error it would otherwise report has nowhere to show. + blankVideo: '', }) return () => { diff --git a/client/vite.config.js b/client/vite.config.js index 931ecd203..0be595f4d 100644 --- a/client/vite.config.js +++ b/client/vite.config.js @@ -2,7 +2,7 @@ import react from '@vitejs/plugin-react'; import { defineConfig } from 'vite'; import { VitePWA } from 'vite-plugin-pwa'; import { visualizer } from 'rollup-plugin-visualizer'; -import { rtlTextAlias } from './rtlTextAlias.js'; +import { rtlTextAlias, plyrSpriteAlias } from './rtlTextAlias.js'; import { readFileSync } from 'node:fs'; // The version this bundle is built as, baked in at build time. The release image @@ -259,7 +259,7 @@ export default defineConfig(({ mode }) => ({ }, }), ].filter(Boolean), - resolve: { alias: [rtlTextAlias] }, + resolve: { alias: [rtlTextAlias, plyrSpriteAlias] }, build: { // Pin the output level instead of inheriting whatever the current Vite default // is, so a toolchain bump can't silently change which browsers still work. diff --git a/client/vitest.config.ts b/client/vitest.config.ts index 27b403c27..531097d32 100644 --- a/client/vitest.config.ts +++ b/client/vitest.config.ts @@ -1,6 +1,6 @@ import { defineConfig } from 'vitest/config'; import react from '@vitejs/plugin-react'; -import { rtlTextAlias } from './rtlTextAlias.js'; +import { rtlTextAlias, plyrSpriteAlias } from './rtlTextAlias.js'; import { readFileSync } from 'node:fs'; const UI_VERSION = JSON.parse(readFileSync(new URL('./package.json', import.meta.url), 'utf8')).version; @@ -9,7 +9,7 @@ export default defineConfig({ define: { __TREK_UI_VERSION__: JSON.stringify(UI_VERSION) }, plugins: [react()], test: { - alias: [rtlTextAlias], + alias: [rtlTextAlias, plyrSpriteAlias], root: '.', globals: true, environment: './tests/environment/jsdom-native-abort.ts',