collective
Browse / github.com/storytold/artcraft / patch
fix(video-editor): preserve live media URLs across undo
fix(video-editor): preserve live media URLs across undo ## Problem Undoing a multi-asset deletion can restore revoked object URLs. Each removal command saves the whole media array. Undo recreates a valid URL for its own asset, but a later undo replaces the whole array with an older snapshot containing the already-revoked URL of another restored asset. The same problem occurs with consecutive individual removals. The asset bin calls the real `MediaManager.removeMediaAssets` path; this leaves restored media unreadable despite appearing in the bin. ## Change Restore only the removed asset into the current media list at its saved position. Preserve
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- rudycelekli (imported from pull request #2084, open)
- Base
28a39a0de4a2fc2eccc237df2046c24245bd12ab- Size
- +94 −5 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
14f217566575d6e1cbb998d0
How it went
No other operator has reported using it yet.
Diff
raw · frontend/libs/components/video-editor/src/lib/commands/media/remove-media-asset.ts frontend/libs/components/video-editor/src/lib/core/managers/media-undo.spec.ts
@@ -91,11 +91,15 @@ export class RemoveMediaAssetCommand extends Command { url: URL.createObjectURL(this.removedAsset.file), }; - editor.media.setAssets({- assets: this.savedAssets.map((a) =>- a.id === this.assetId ? restoredAsset : a,- ),- });+ // Other removals may already have been undone, recreating their+ // object URLs. Replacing the whole saved array would restore their+ // revoked URLs instead. Restore only this asset into the live list.+ const restoredAssets = [...editor.media.getAssets()];+ const savedIndex = this.savedAssets.findIndex(+ (asset) => asset.id === this.assetId,+ );+ restoredAssets.splice(savedIndex, 0, restoredAsset);+ editor.media.setAssets({ assets: restoredAssets }); // Persistence handled by host via ProjectStorageAdapter }new file mode 100644@@ -0,0 +1,85 @@+// @vitest-environment node+import { MediaManager } from "./media-manager";+import { CommandManager } from "./commands";+import type { EditorCore } from "../index";+import type { SceneTracks } from "../../timeline/types";+import type { MediaAsset } from "../../media/types";++const fixture = vi.hoisted(() => ({ editor: null as unknown as EditorCore }));+vi.mock("../index", () => ({ EditorCore: { getInstance: () => fixture.editor } }));+vi.mock("../../commands", async () => await import("../../commands/batch-command"));+vi.mock("../../commands/media", async () => await import("../../commands/media/remove-media-asset"));+// These tests remove assets from an empty timeline. Rendering caches and+// unused ripple/element-construction code are excluded from this boundary.+vi.mock("../../services/video-cache", () => ({ videoCache: { clearVideo: vi.fn(), clearAll: vi.fn() } }));+vi.mock("../../services/video-cache/service", () => ({ videoCache: { clearVideo: vi.fn(), clearAll: vi.fn() } }));+vi.mock("../../services/waveform-cache", () => ({ waveformCache: { clearSource: vi.fn(), clearAll: vi.fn() } }));+vi.mock("../../services/waveform-cache/service", () => ({ waveformCache: { clearSource: vi.fn(), clearAll: vi.fn() } }));+vi.mock("../../retime", () => ({ getSourceTimeAtClipTime: () => { throw new Error("waveform rendering not used"); } }));+vi.mock("../../ripple", () => ({+ applyRippleAdjustments: () => { throw new Error("ripple is disabled"); },+ computeRippleAdjustments: () => { throw new Error("ripple is disabled"); },+}));+vi.mock("../../timeline/element-utils", () => ({ hasMediaId: () => { throw new Error("timeline has no elements"); } }));++afterEach(() => fixture.editor.media.clearAllAssets());++it("keeps both files readable after undoing a multi-asset removal", async () => {+ const { media, command } = buildFixture();+ media.removeMediaAssets({ projectId: "project", ids: ["a", "b"] });+ expect(media.getAssets().map((asset) => asset.id)).toEqual(["c"]);+ command.undo();+ expect(media.getAssets().map((asset) => asset.id)).toEqual(["a", "b", "c"]);+ expect(await readAssets(media)).toEqual(["contents-a", "contents-b", "contents-c"]);+});++it("preserves an already-restored file when undoing consecutive removals", async () => {+ const { media, command } = buildFixture();+ media.removeMediaAsset({ projectId: "project", id: "a" });+ media.removeMediaAsset({ projectId: "project", id: "b" });+ command.undo();+ command.undo();+ expect(await readAssets(media)).toEqual(["contents-a", "contents-b", "contents-c"]);+});++it("supports repeated batch undo/redo without restoring revoked URLs", async () => {+ const { media, command } = buildFixture();+ media.removeMediaAssets({ projectId: "project", ids: ["a", "b"] });+ for (let cycle = 0; cycle < 3; cycle++) {+ command.undo();+ expect(await readAssets(media)).toEqual(["contents-a", "contents-b", "contents-c"]);+ command.redo();+ expect(media.getAssets().map((asset) => asset.id)).toEqual(["c"]);+ }+});++it("preserves the ordinary single removal and undo path", async () => {+ const { media, command } = buildFixture();+ media.removeMediaAsset({ projectId: "project", id: "b" });+ expect(await readAssets(media)).toEqual(["contents-a", "contents-c"]);+ command.undo();+ expect(await readAssets(media)).toEqual(["contents-a", "contents-b", "contents-c"]);+});++function buildFixture() {+ let tracks = { overlay: [], main: { id: "main", elements: [] }, audio: [] } as unknown as SceneTracks;+ const editor = {+ scenes: { getActiveScene: () => ({ tracks }) },+ timeline: { updateTracks: (next: SceneTracks) => { tracks = next; } },+ selection: { getSnapshot: () => ({}) },+ } as unknown as EditorCore;+ const media = new MediaManager(editor);+ const command = new CommandManager(editor);+ Object.assign(editor, { media, command });+ fixture.editor = editor;+ const assets: MediaAsset[] = ["a", "b", "c"].map((id) => {+ const file = new File([`contents-${id}`], `${id}.mp4`, { type: "video/mp4" });+ return { id, file, name: id, type: "video", url: URL.createObjectURL(file) };+ });+ media.loadProjectMedia({ assets });+ return { media, command };+}++async function readAssets(media: MediaManager) {+ return Promise.all(media.getAssets().map(async (asset) => (await fetch(asset.url!)).text()));+}