diff --git a/packages/studio/src/hooks/useDomEditCommits.test.tsx b/packages/studio/src/hooks/useDomEditCommits.test.tsx index 8761f2903f..6e118c35c8 100644 --- a/packages/studio/src/hooks/useDomEditCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditCommits.test.tsx @@ -228,7 +228,9 @@ function renderDomEditCommits( applyDomSelection: vi.fn(), clearDomSelection: vi.fn(), refreshDomEditSelectionFromPreview: vi.fn(), - buildDomSelectionFromTarget: vi.fn(async () => null), + buildDomSelectionFromTarget: vi.fn(async (target: HTMLElement) => + target === selection.element ? selection : null, + ), onTrySdkPersist: options.onTrySdkPersist, readOnlyPreview: false, }); @@ -893,7 +895,7 @@ describe("useDomEditCommits rich-text persist handling", () => { } }); - it("does not retarget a commit onto a replacement preview node", async () => { + it("does not retarget a commit onto a replacement preview node, and says the text was not saved", async () => { const fetchMock = stubPatchFetch({ ok: true, changed: true, matched: true }); const html = 'After'; const { iframe, element } = createPreviewElement(`
${html}
`); @@ -902,6 +904,7 @@ describe("useDomEditCommits rich-text persist handling", () => { replacement.dataset.hfId = "hf-card"; replacement.innerHTML = "Reloaded elsewhere"; element.replaceWith(replacement); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); try { await act(async () => { @@ -914,8 +917,14 @@ describe("useDomEditCommits rich-text persist handling", () => { expect(replacement.innerHTML).toBe("Reloaded elsewhere"); expect(fetchMock).not.toHaveBeenCalled(); - expect(rendered.showToast).not.toHaveBeenCalled(); + expect(rendered.showToast).toHaveBeenCalledWith( + expect.stringMatching( + /Couldn't save the text edit: the text's element is gone from the preview/, + ), + "error", + ); } finally { + errorSpy.mockRestore(); rendered.cleanup(); } }); diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index eacf065279..fa36a4f12a 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -7,6 +7,7 @@ import { StudioSaveHttpError, trackStudioSaveFailure } from "../utils/studioSave import type { DomEditSelection } from "../components/editor/domEditing"; import { fontFamilyFromAssetPath, type ImportedFontAsset } from "../components/editor/fontAssets"; import type { CommitDomEditPatchBatches } from "./domEditCommitTypes"; +import type { ResolveDomSelectionOptions } from "./useDomSelectionTypes"; import type { PatchOperation } from "../utils/sourcePatcher"; import { DomEditPersistUnsafeValueError } from "./domEditPersistFailure"; import { useDomEditPersist, type RecordEditInput } from "./useDomEditPersist"; @@ -46,7 +47,7 @@ export interface UseDomEditCommitsParams { refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => void; buildDomSelectionFromTarget: ( target: HTMLElement, - options?: { preferClipAncestor?: boolean }, + options?: ResolveDomSelectionOptions, ) => Promise; /** Resync the in-memory SDK session after a SERVER-side write (NOT the SDK * path, whose session is already current) so a later SDK edit doesn't diff --git a/packages/studio/src/hooks/useDomEditTextCommits.test.tsx b/packages/studio/src/hooks/useDomEditTextCommits.test.tsx index 4adaec4c68..3057120c54 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditTextCommits.test.tsx @@ -125,13 +125,20 @@ afterEach(() => { }); describe("useDomEditTextCommits", () => { - function richTextProbe() { - const { iframe, element } = previewElement('

Old

', "t"); + function richTextProbe( + html = '

Old

', + overrides: (doc: Document) => Partial = () => ({}), + ) { + const { iframe, element } = previewElement(html, "t"); const persist = vi.fn().mockResolvedValue(undefined); + const showToast = vi.fn(); const base = commitParams({ previewIframeRef: { current: iframe }, domEditSelection: selectionFor(element), + buildDomSelectionFromTarget: vi.fn(async (target: HTMLElement) => selectionFor(target)), persistDomEditOperations: persist, + showToast, + ...overrides(element.ownerDocument), }); const captured: { hook: ReturnType | null } = { hook: null }; function Probe({ readOnlyPreview }: { readOnlyPreview: boolean }) { @@ -143,13 +150,25 @@ describe("useDomEditTextCommits", () => { const save = captured.hook!.handleDomRichTextCommit; const commit = { element, html: "New", previousHtml: "Old" }; element.innerHTML = "New"; - return { root, Probe, persist, element, save: () => act(async () => save(commit)) }; + return { + root, + Probe, + persist, + showToast, + element, + save: () => act(async () => save(commit)), + }; } - it("saves in-place text while the preview is editable", async () => { - const { persist, save } = richTextProbe(); + it("saves in-place text while the preview is editable, and refreshes the selection it edited", async () => { + const applyDomSelection = vi.fn(); + const { persist, element, save } = richTextProbe(undefined, () => ({ applyDomSelection })); await save(); expect(persist).toHaveBeenCalledTimes(1); + expect(applyDomSelection).toHaveBeenCalledWith( + expect.objectContaining({ element }), + expect.objectContaining({ preserveGroup: true }), + ); }); it("refuses in-place text once the preview turns read-only, through an earlier handler, and puts the old text back", async () => { @@ -160,6 +179,44 @@ describe("useDomEditTextCommits", () => { expect(element.innerHTML).toBe("Old"); }); + it("saves the edited element, and leaves the selection alone, when the selection is another element or none", async () => { + for (const selected of ["card", null]) { + const applyDomSelection = vi.fn(); + const { persist, element, save } = richTextProbe( + '

Old

', + (doc) => ({ + domEditSelection: selected ? selectionFor(doc.getElementById(selected)!) : null, + applyDomSelection, + buildDomSelectionFromTarget: vi.fn(async (target: HTMLElement) => ({ + ...selectionFor(target), + label: "Resolved from the edited element", + })), + }), + ); + await save(); + expect(persist).toHaveBeenCalledTimes(1); + expect(persist.mock.calls[0]![0]).toMatchObject({ + element, + label: "Resolved from the edited element", + }); + expect(applyDomSelection).not.toHaveBeenCalled(); + cleanup?.(); + cleanup = null; + } + }); + + it("says so and puts the old text back when the edited text cannot be saved", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const { persist, showToast, element, save } = richTextProbe(undefined, () => ({ + buildDomSelectionFromTarget: vi.fn(async () => null), + })); + await save(); + expect(persist).not.toHaveBeenCalled(); + expect(showToast).toHaveBeenCalledWith(expect.stringContaining("Couldn't save"), "error"); + expect(error).toHaveBeenCalled(); + expect(element.innerHTML).toBe("Old"); + }); + it("keeps concurrent text commit ownership isolated by target", async () => { const { iframe, element: firstElement } = previewElement( "
First
Second
", diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index 83c3af5c90..4ec9055cd3 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -29,6 +29,7 @@ import { import { commitDomStyles } from "./domStyleCommit"; import { useDomEditAttributeCommits } from "./useDomEditAttributeCommits"; import type { InlineTextEditCommit } from "./useInlineTextEdit"; +import type { ResolveDomSelectionOptions } from "./useDomSelectionTypes"; // ── Types ── @@ -44,7 +45,7 @@ export interface UseDomEditTextCommitsParams { refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => void; buildDomSelectionFromTarget: ( target: HTMLElement, - options?: { preferClipAncestor?: boolean }, + options?: ResolveDomSelectionOptions, ) => Promise; persistDomEditOperations: PersistDomEditOperations; resolveImportedFontAsset: (fontFamilyValue: string) => ImportedFontAsset | null; @@ -56,15 +57,6 @@ function canCommitInlineTextSelection(selection: DomEditSelection, element: HTML return canEditElementTextInline(element); } -function ownsCurrentPreviewElement( - selection: DomEditSelection, - element: HTMLElement, - document: Document | null | undefined, -): document is Document { - if (!document || !element.isConnected) return false; - return element === selection.element && element.ownerDocument === document; -} - async function resyncDomTextSelectionFromPreview( doc: Document | null | undefined, selection: DomEditSelection, @@ -96,6 +88,8 @@ export function useDomEditTextCommits({ }: UseDomEditTextCommitsParams) { const latestReadOnlyPreviewRef = useRef(readOnlyPreview); latestReadOnlyPreviewRef.current = readOnlyPreview; + const latestSelectionRef = useRef(domEditSelection); + latestSelectionRef.current = domEditSelection; const domTextCommitVersionRef = useRef(new Map()); const domStyleCommitVersionRef = useRef(new Map()); @@ -238,29 +232,36 @@ export function useDomEditTextCommits({ */ const handleDomRichTextCommit = useCallback( async ({ element, html, previousHtml }: InlineTextEditCommit) => { - if (!domEditSelection) return; - if (latestReadOnlyPreviewRef.current) { + const putBack = () => { if (element.isConnected && element.innerHTML === html) element.innerHTML = previousHtml; - return; + }; + if (latestReadOnlyPreviewRef.current) return putBack(); + const refuse = (reason: string) => { + console.error("[Studio] text edit not saved:", reason, element); + showToast(`Couldn't save the text edit: ${reason}`, "error"); + putBack(); + }; + // The edited node, not the current selection: a press can open an edit on a child of what + // is selected, and a host can clear the selection before the edit closes. + const doc = previewIframeRef.current?.contentDocument; + if (!doc || !element.isConnected || element.ownerDocument !== doc) { + return refuse("the text's element is gone from the preview"); + } + const selection = await buildDomSelectionFromTarget(element, { + exactTarget: true, + skipSourceProbe: true, + }); + if (selection?.element !== element) { + return refuse("this text was not found in the composition's source"); + } + // The same gate that let the edit open, not the design panel's field rule: an element + // whose text holds a line break has no fields, and editing in place rewrites its markup. + if (!canCommitInlineTextSelection(selection, element)) { + return refuse("this text can't be edited in place"); } - // The same gate that let the edit open, not the design panel's. - // - // The panel's rule is about its text fields, and it has none for an - // element whose text contains a line break: a `` holding `
`s - // is not a leaf, so nothing inside is a field and the element reports no - // editable text at all. Editing in place does not use fields — it - // rewrites the element's own markup — so refusing on that rule refused - // elements the caret had just been opened in, and every colour the user - // chose was dropped on the way out with nothing said about it. - if (!canCommitInlineTextSelection(domEditSelection, element)) return; - const iframe = previewIframeRef.current; - const doc = iframe?.contentDocument; - // A preview reload replaces the document. Never resolve this commit onto - // the replacement node: it did not own the edit or its rollback snapshot. - if (!ownsCurrentPreviewElement(domEditSelection, element, doc)) return; const isLatestTextCommit = bumpDomEditCommitMapVersion( domTextCommitVersionRef.current, - getDomEditTargetKey(domEditSelection), + getDomEditTargetKey(selection), ); const operations = [buildDomEditRichTextPatchOperation(html)]; let appliedHtml = ""; @@ -274,7 +275,7 @@ export function useDomEditTextCommits({ appliedHtml = element.innerHTML; }, persist: async () => { - await persistDomEditOperations(domEditSelection, operations, { + await persistDomEditOperations(selection, operations, { label: "Edit text", skipRefresh: true, shouldSave: isLatestTextCommit, @@ -288,13 +289,13 @@ export function useDomEditTextCommits({ element.innerHTML = previousHtml; } }, - onError: (error) => - reportDomEditPersistFailure(domEditSelection, operations, error, showToast), - shouldResync: isLatestTextCommit, + onError: (error) => reportDomEditPersistFailure(selection, operations, error, showToast), + // Re-select only what is still selected: the selection may have moved on, or a host cleared it. + shouldResync: () => isLatestTextCommit() && latestSelectionRef.current?.element === element, resync: () => resyncDomTextSelectionFromPreview( doc, - domEditSelection, + selection, activeCompPath, buildDomSelectionFromTarget, applyDomSelection, @@ -306,7 +307,6 @@ export function useDomEditTextCommits({ activeCompPath, applyDomSelection, buildDomSelectionFromTarget, - domEditSelection, persistDomEditOperations, previewIframeRef, showToast,