From 9a4c5155a9bcc4f8dd97f5e64ef559c86e4b39d6 Mon Sep 17 00:00:00 2001 From: webadderall <131426131+webadderall@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:59:09 +1000 Subject: [PATCH] fix(editor): address PR review feedback --- electron/ipc/register/project.ts | 76 ++++++++++++++++++++- src/components/video-editor/VideoEditor.tsx | 27 ++++++-- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/electron/ipc/register/project.ts b/electron/ipc/register/project.ts index 9310dc2d..e3fbd502 100644 --- a/electron/ipc/register/project.ts +++ b/electron/ipc/register/project.ts @@ -41,6 +41,9 @@ function normalizeRecordingTimeOffsetMs(value: unknown): number { return typeof value === "number" && Number.isFinite(value) ? Math.round(value) : 0; } +/** + * Produces a filesystem-safe project base name without the project extension. + */ function normalizeProjectSaveName(projectName?: string | null) { if (typeof projectName !== "string") { return null; @@ -55,8 +58,11 @@ function normalizeProjectSaveName(projectName?: string | null) { new RegExp(`\\.${PROJECT_FILE_EXTENSION}$`, "i"), "", ); - const sanitizedName = withoutExtension - .replace(/[<>:"/\\|?*\u0000-\u001F]/g, "") + const withoutInvalidFilesystemChars = withoutExtension.replace(/[<>:"/\\|?*]/g, ""); + const withoutControlChars = Array.from(withoutInvalidFilesystemChars) + .filter((character) => character.charCodeAt(0) > 31) + .join(""); + const sanitizedName = withoutControlChars .replace(/\s+/g, " ") .replace(/[. ]+$/g, "") .trim(); @@ -64,6 +70,64 @@ function normalizeProjectSaveName(projectName?: string | null) { return sanitizedName || null; } +/** + * Extracts the persisted source video path from a saved project payload. + */ +function getProjectVideoPath(projectData: unknown) { + if (!projectData || typeof projectData !== "object") { + return null; + } + + const candidate = projectData as { videoPath?: unknown }; + return typeof candidate.videoPath === "string" ? candidate.videoPath : null; +} + +/** + * Prevents a named save from silently overwriting a different project file. + */ +async function ensureNamedProjectSaveDoesNotOverwriteDifferentProject( + targetProjectPath: string, + projectData: unknown, +) { + try { + await fs.stat(targetProjectPath); + } catch (error) { + if ((error as NodeJS.ErrnoException)?.code === "ENOENT") { + return { success: true }; + } + throw error; + } + + const incomingVideoPath = getProjectVideoPath(projectData); + if (!incomingVideoPath) { + return { + success: false, + message: "Unable to verify project identity for the chosen name", + }; + } + + try { + const existingProjectRaw = await fs.readFile(targetProjectPath, "utf-8"); + const existingProjectData = JSON.parse(existingProjectRaw) as unknown; + const existingVideoPath = getProjectVideoPath(existingProjectData); + + if (existingVideoPath === incomingVideoPath) { + return { success: true }; + } + } catch (error) { + console.error("Failed to verify existing named project before overwrite:", error); + return { + success: false, + message: "A different project already uses this name", + }; + } + + return { + success: false, + message: "A different project already uses this name", + }; +} + export function registerProjectHandlers() { ipcMain.handle('reveal-in-folder', async (_, filePath: string) => { try { @@ -222,6 +286,14 @@ export function registerProjectHandlers() { `${normalizedProjectName}.${PROJECT_FILE_EXTENSION}`, ) + const overwriteCheck = await ensureNamedProjectSaveDoesNotOverwriteDifferentProject( + targetProjectPath, + projectData, + ) + if (!overwriteCheck.success) { + return overwriteCheck + } + await fs.writeFile(targetProjectPath, JSON.stringify(projectData, null, 2), 'utf-8') setCurrentProjectPath(targetProjectPath) await saveProjectThumbnail(targetProjectPath, thumbnailDataUrl) diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index ffe7e4c9..f8dc0597 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -2202,6 +2202,9 @@ export default function VideoEditor() { } }, [saveProject]); + /** + * Saves the current project directly into the projects library under a chosen name. + */ const saveProjectWithName = useCallback( async (projectName: string) => { const trimmedProjectName = projectName.trim(); @@ -2258,11 +2261,17 @@ export default function VideoEditor() { ], ); + /** + * Resets the inline project-name editor back to the current saved display name. + */ const closeProjectNameEditor = useCallback(() => { setProjectNameDraft(projectDisplayName); setIsEditingProjectName(false); }, [projectDisplayName]); + /** + * Commits the inline project-name editor and persists the project under that name. + */ const handleProjectNameSubmit = useCallback( async (event?: React.FormEvent) => { event?.preventDefault(); @@ -2273,8 +2282,12 @@ export default function VideoEditor() { } setIsSavingProjectName(true); - const saved = await saveProjectWithName(trimmedProjectName); - setIsSavingProjectName(false); + let saved = false; + try { + saved = await saveProjectWithName(trimmedProjectName); + } finally { + setIsSavingProjectName(false); + } if (saved) { setIsEditingProjectName(false); @@ -2922,19 +2935,19 @@ export default function VideoEditor() { if (deletedClip) { const { startMs, endMs } = deletedClip; setZoomRegions((prev) => - prev.filter((region) => region.startMs < startMs || region.endMs > endMs), + prev.filter((region) => region.endMs <= startMs || region.startMs >= endMs), ); setAnnotationRegions((prev) => - prev.filter((region) => region.startMs < startMs || region.endMs > endMs), + prev.filter((region) => region.endMs <= startMs || region.startMs >= endMs), ); setTrimRegions((prev) => - prev.filter((region) => region.startMs < startMs || region.endMs > endMs), + prev.filter((region) => region.endMs <= startMs || region.startMs >= endMs), ); setSpeedRegions((prev) => - prev.filter((region) => region.startMs < startMs || region.endMs > endMs), + prev.filter((region) => region.endMs <= startMs || region.startMs >= endMs), ); setAudioRegions((prev) => - prev.filter((region) => region.startMs < startMs || region.endMs > endMs), + prev.filter((region) => region.endMs <= startMs || region.startMs >= endMs), ); } if (selectedClipId === id) {