From b6d1f63e01ce62b4af0801aa8e4c167bcb0aa0ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yi=C4=9Fit?= <75206391+yigitozen33@users.noreply.github.com> Date: Sat, 24 Oct 2026 05:21:44 +0300 Subject: [PATCH] fix(editor): resume media previews from their last playback position (#32480) * fix(editor): resume media previews from their last playback position Switching tabs unmounts the editor surface, so the media element was recreated and every video or audio preview restarted from zero. Record the playback time on teardown and seek back once metadata loads. Co-Authored-By: Claude Opus 5.5 * fix(editor): resume media previews from their last playback position Switching tabs unmounts the editor surface, so the media element was recreated and every video or audio preview restarted from zero. Record the playback time on teardown and seek back once metadata loads. Co-Authored-By: Claude Opus 5.5 * test(editor): verify media remount playback and cleanup --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- .../editor/EditorEditFileSurface.tsx | 1 + .../components/editor/MediaViewer.test.tsx | 95 ++++++++++++++++++- .../src/components/editor/MediaViewer.tsx | 31 ++++-- src/renderer/src/lib/scroll-cache.ts | 4 + .../editor-media-playback-position.spec.ts | 89 +++++++++++++++++ 5 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 tests/e2e/editor-media-playback-position.spec.ts diff --git a/src/renderer/src/components/editor/EditorEditFileSurface.tsx b/src/renderer/src/components/editor/EditorEditFileSurface.tsx index 4e5a44fae76..939b9cb73be 100644 --- a/src/renderer/src/components/editor/EditorEditFileSurface.tsx +++ b/src/renderer/src/components/editor/EditorEditFileSurface.tsx @@ -131,6 +131,7 @@ export function EditorEditFileSurface({ src={fileContent.mediaUrl} mimeType={fileContent.mimeType ?? 'video/mp4'} filePath={activeFile.filePath} + playbackCacheKey={pdfViewStateKey} canOpenLocally={ !activeFile.externalSshTargetId && !activeFile.runtimeEnvironmentId && diff --git a/src/renderer/src/components/editor/MediaViewer.test.tsx b/src/renderer/src/components/editor/MediaViewer.test.tsx index 7d87eddf735..b75868b2f96 100644 --- a/src/renderer/src/components/editor/MediaViewer.test.tsx +++ b/src/renderer/src/components/editor/MediaViewer.test.tsx @@ -1,14 +1,45 @@ // @vitest-environment happy-dom import { StrictMode, act } from 'react' -import { createRoot } from 'react-dom/client' +import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' +import { mediaPlaybackPositionCache } from '@/lib/scroll-cache' import MediaViewer from './MediaViewer' vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) describe('MediaViewer', () => { - afterEach(() => vi.restoreAllMocks()) + afterEach(() => { + vi.restoreAllMocks() + mediaPlaybackPositionCache.clear() + }) + + const renderPlaybackViewer = async ( + playbackCacheKey: string, + duration = 60 + ): Promise<{ root: Root; video: HTMLVideoElement }> => { + const container = document.createElement('div') + const root = createRoot(container) + await act(async () => + root.render( + + + + ) + ) + const video = container.querySelector('video') + if (!video) { + throw new Error('video not rendered') + } + Object.defineProperty(video, 'duration', { configurable: true, value: duration }) + return { root, video } + } it('keeps its source through Strict Mode setup and stops playback on unmount', async () => { const pause = vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}) @@ -40,6 +71,66 @@ describe('MediaViewer', () => { expect(video?.getAttribute('src')).toBeNull() }) + it('resumes from the last playback position after the tab remounts', async () => { + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}) + vi.spyOn(HTMLMediaElement.prototype, 'load').mockImplementation(() => {}) + const first = await renderPlaybackViewer('clip.mp4:pdf') + first.video.currentTime = 42 + await act(async () => first.root.unmount()) + + const second = await renderPlaybackViewer('clip.mp4:pdf') + second.video.currentTime = 0 + await act(async () => second.video.dispatchEvent(new Event('loadedmetadata'))) + expect(second.video.currentTime).toBe(42) + await act(async () => second.root.unmount()) + }) + + it('retains the position when leaving before metadata and clamps it when the file shrinks', async () => { + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}) + vi.spyOn(HTMLMediaElement.prototype, 'load').mockImplementation(() => {}) + const first = await renderPlaybackViewer('clip.mp4:pdf') + first.video.currentTime = 42 + await act(async () => first.root.unmount()) + + const loading = await renderPlaybackViewer('clip.mp4:pdf', Number.NaN) + await act(async () => loading.root.unmount()) + + const shorter = await renderPlaybackViewer('clip.mp4:pdf', 20) + await act(async () => shorter.video.dispatchEvent(new Event('loadedmetadata'))) + expect(shorter.video.currentTime).toBe(20) + await act(async () => shorter.root.unmount()) + }) + + it('keeps separate view positions and restarts a completed clip', async () => { + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}) + vi.spyOn(HTMLMediaElement.prototype, 'load').mockImplementation(() => {}) + const firstPane = await renderPlaybackViewer('clip.mp4::pane-a:pdf') + firstPane.video.currentTime = 12 + await act(async () => firstPane.root.unmount()) + + const otherPane = await renderPlaybackViewer('clip.mp4::pane-b:pdf') + await act(async () => otherPane.video.dispatchEvent(new Event('loadedmetadata'))) + expect(otherPane.video.currentTime).toBe(0) + otherPane.video.currentTime = 27 + await act(async () => otherPane.root.unmount()) + + const restored = await renderPlaybackViewer('clip.mp4::pane-a:pdf') + await act(async () => restored.video.dispatchEvent(new Event('loadedmetadata'))) + expect(restored.video.currentTime).toBe(12) + Object.defineProperty(restored.video, 'ended', { configurable: true, value: true }) + await act(async () => restored.root.unmount()) + + const completed = await renderPlaybackViewer('clip.mp4::pane-a:pdf') + await act(async () => completed.video.dispatchEvent(new Event('loadedmetadata'))) + expect(completed.video.currentTime).toBe(0) + await act(async () => completed.root.unmount()) + + const otherRestored = await renderPlaybackViewer('clip.mp4::pane-b:pdf') + await act(async () => otherRestored.video.dispatchEvent(new Event('loadedmetadata'))) + expect(otherRestored.video.currentTime).toBe(27) + await act(async () => otherRestored.root.unmount()) + }) + it('shows a persistent playback error and only offers the default app for local files', async () => { vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}) vi.spyOn(HTMLMediaElement.prototype, 'load').mockImplementation(() => {}) diff --git a/src/renderer/src/components/editor/MediaViewer.tsx b/src/renderer/src/components/editor/MediaViewer.tsx index c89fd1d5828..9ac33d820c2 100644 --- a/src/renderer/src/components/editor/MediaViewer.tsx +++ b/src/renderer/src/components/editor/MediaViewer.tsx @@ -2,36 +2,53 @@ import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' import { useEffect, useRef, useState } from 'react' import { translate } from '@/i18n/i18n' import { Button } from '@/components/ui/button' +import { mediaPlaybackPositionCache, setWithLRU } from '@/lib/scroll-cache' export default function MediaViewer({ src, mimeType, filePath, - canOpenLocally + canOpenLocally, + playbackCacheKey = null }: { src: string mimeType: string filePath: string canOpenLocally: boolean + playbackCacheKey?: string | null }): React.JSX.Element { const mediaRef = useRef(null) const [failed, setFailed] = useState(false) const [openFailed, setOpenFailed] = useState(false) useEffect(() => { const media = mediaRef.current - // Strict Mode replays cleanup while retaining the DOM element. - if (media) { - media.src = src + if (!media) { + return } + const restorePosition = (): void => { + const saved = playbackCacheKey ? mediaPlaybackPositionCache.get(playbackCacheKey) : undefined + if (saved !== undefined && Number.isFinite(media.duration)) { + media.currentTime = Math.min(saved, media.duration) + } + } + media.addEventListener('loadedmetadata', restorePosition, { once: true }) + // Strict Mode replays cleanup while retaining the DOM element. + media.src = src return () => { - if (!media) { - return + media.removeEventListener('loadedmetadata', restorePosition) + // Why: before metadata loads, currentTime is 0 and would clobber the saved position. + if (playbackCacheKey && Number.isFinite(media.duration)) { + if (media.ended) { + mediaPlaybackPositionCache.delete(playbackCacheKey) + } else { + setWithLRU(mediaPlaybackPositionCache, playbackCacheKey, media.currentTime) + } } media.pause() media.removeAttribute('src') media.load() } - }, [src]) + }, [src, playbackCacheKey]) const playerProps = { ref: (element: HTMLMediaElement | null): void => { diff --git a/src/renderer/src/lib/scroll-cache.ts b/src/renderer/src/lib/scroll-cache.ts index de28893ebaa..4201e07bea2 100644 --- a/src/renderer/src/lib/scroll-cache.ts +++ b/src/renderer/src/lib/scroll-cache.ts @@ -46,6 +46,10 @@ export const editorSelectionCache = new Map() export type PdfViewPosition = { pageNumber: number; top: number; left: number } export const pdfViewPositionCache = new Map() +// Why: switching tabs unmounts the media element, so playback time (seconds) +// must outlive it or the clip restarts from zero on return. +export const mediaPlaybackPositionCache = new Map() + // Why: Diff editors need more than a numeric scroll offset to restore the same // working context. Monaco's diff view state also carries cursor/selection state // for both sides plus diff model state, which matches VS Code's restore path diff --git a/tests/e2e/editor-media-playback-position.spec.ts b/tests/e2e/editor-media-playback-position.spec.ts new file mode 100644 index 00000000000..4cdc2901f0b --- /dev/null +++ b/tests/e2e/editor-media-playback-position.spec.ts @@ -0,0 +1,89 @@ +import { copyFileSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { test, expect } from './helpers/orca-app' +import { openFileExplorer } from './helpers/file-explorer' +import { waitForSessionReady } from './helpers/store' + +test.use({ seedTestRepo: false }) + +test('restores media playback after switching editor tabs without retaining the player', async ({ + orcaPage, + registerPostElectronShutdownCleanup +}) => { + const folder = realpathSync(mkdtempSync(path.join(tmpdir(), 'orca-media-position-'))) + registerPostElectronShutdownCleanup(async () => rmSync(folder, { recursive: true, force: true })) + copyFileSync( + path.resolve('docs/site/public/docs/videos/tab-split.mp4'), + path.join(folder, 'playback.mp4') + ) + writeFileSync(path.join(folder, 'other.txt'), 'Switch back to the media tab.\n') + + await waitForSessionReady(orcaPage) + await orcaPage.evaluate(async (folder) => { + await window.__store?.getState().addRepoPath(folder, 'folder') + }, folder) + await orcaPage + .getByRole('option') + .filter({ hasText: path.basename(folder) }) + .click() + await openFileExplorer(orcaPage) + const row = (name: string) => + orcaPage.locator('[data-file-explorer-row]').filter({ + has: orcaPage.locator('[data-file-explorer-row-name]').getByText(name, { exact: true }) + }) + await row('playback.mp4').dblclick() + const video = orcaPage.locator('video') + await expect + .poll(() => video.evaluate((media: HTMLVideoElement) => media.readyState)) + .toBeGreaterThanOrEqual(2) + await video.evaluate(async (media: HTMLVideoElement) => { + media.muted = true + media.currentTime = 1 + await media.play() + }) + await expect + .poll(() => video.evaluate((media: HTMLVideoElement) => media.currentTime)) + .toBeGreaterThan(1.1) + const savedTime = await video.evaluate((media: HTMLVideoElement) => { + media.pause() + return media.currentTime + }) + const previousPlayer = await video.elementHandle() + if (!previousPlayer) { + throw new Error('Media player disappeared before switching tabs') + } + + await row('other.txt').dblclick() + await expect(video).toHaveCount(0) + expect( + await previousPlayer.evaluate((media: HTMLVideoElement) => ({ + connected: media.isConnected, + paused: media.paused, + source: media.getAttribute('src') + })) + ).toEqual({ connected: false, paused: true, source: null }) + + await orcaPage.locator('[data-tab-id]').filter({ hasText: 'playback.mp4' }).click() + await expect + .poll(() => video.evaluate((media: HTMLVideoElement) => media.readyState)) + .toBeGreaterThanOrEqual(2) + expect(await video.evaluate((media: HTMLVideoElement) => media.currentTime)).toBeCloseTo( + savedTime, + 1 + ) + expect( + await video.evaluate((media: HTMLVideoElement) => ({ + paused: media.paused, + controls: media.controls + })) + ).toEqual({ + paused: true, + controls: true + }) + await video.evaluate((media: HTMLVideoElement) => media.play()) + await expect + .poll(() => video.evaluate((media: HTMLVideoElement) => media.currentTime)) + .toBeGreaterThan(savedTime + 0.1) + await video.evaluate((media: HTMLVideoElement) => media.pause()) +})