mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +00:00
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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * test(editor): verify media remount playback and cleanup --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
Neil
parent
c1d980c111
commit
b6d1f63e01
@@ -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 &&
|
||||
|
||||
@@ -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(
|
||||
<StrictMode>
|
||||
<MediaViewer
|
||||
src="orca-media://file/token"
|
||||
mimeType="video/mp4"
|
||||
filePath="clip.mp4"
|
||||
canOpenLocally
|
||||
playbackCacheKey={playbackCacheKey}
|
||||
/>
|
||||
</StrictMode>
|
||||
)
|
||||
)
|
||||
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(() => {})
|
||||
|
||||
@@ -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<HTMLMediaElement>(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 => {
|
||||
|
||||
@@ -46,6 +46,10 @@ export const editorSelectionCache = new Map<string, readonly ISelection[]>()
|
||||
export type PdfViewPosition = { pageNumber: number; top: number; left: number }
|
||||
export const pdfViewPositionCache = new Map<string, PdfViewPosition>()
|
||||
|
||||
// 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<string, number>()
|
||||
|
||||
// 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
|
||||
|
||||
@@ -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())
|
||||
})
|
||||
Reference in New Issue
Block a user