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()) +})