Save a page without wrecking it, and spend that on the page list's commands (BL-13502)

.github/skills/bloom-automation/ai-image-editor-driving.md

@@ -62,15 +62,20 @@ Right-click the canvas element → **"Edit with AI…"**. Loading the iframe dir
 `aiImageEditorOverlay.ts`'s postMessage handler, so the commit + current-page save path wouldn't run.
 
 **The overlay does not appear on the same tick as the click.** The menu command posts
-`aiImageEditor/saveThenLaunch`, which makes C# save the current page — reloading the `page`
-frame — and then, *once the reloaded page reports back*, call
+`aiImageEditor/saveThenLaunch`, which makes C# save the current page and then call
 `workspaceBundle.openAiImageEditor(...)` in the shell (BL-16682; the saved book DOM has to be
-current or the editor opens with an empty "Image to Edit" slot). So the wait spans a whole page
-load: **wait for the overlay**, e.g. `await page.waitForSelector("#ai-image-editor-overlay iframe")`,
-and don't re-use any content-frame handle taken before the click — that frame is gone.
-
-Note also that the overlay itself belongs to the **shell**, not the `page` frame, so it survives
-page reloads: a commit saves the page immediately and the ✕ keeps working across that reload.
+current or the editor opens with an empty "Image to Edit" slot). **Wait for the overlay**, e.g.
+`await page.waitForSelector("#ai-image-editor-overlay iframe")`.
+
+That save used to end in a navigation, so the wait spanned a whole page load and any content-frame
+handle taken before the click was stale. Since BL-13502 a save does not navigate: the page frame
+stays put and its handles stay good. Waiting for the overlay is still the right thing — the save
+and the round trip take a moment — but the page is no longer replaced underneath you. (Continuing
+to wait for a page load that no longer happens is precisely what once left the editor never
+opening; see the comment on `HandleSaveThenLaunch`.)
+
+Note also that the overlay itself belongs to the **shell**, not the `page` frame, so it survives a
+reload when one does happen.
 (It does *not* survive a reload of the whole workspace root, which
 `EditingView.StartNavigationToEditPage` does once `MemoryUtils.SystemIsShortOfMemory()` — Bloom's
 own private bytes past ~2GB, i.e. a long session on a big book. That closes the overlay like any

src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts

@@ -44,6 +44,7 @@ import { RenderCanvasElementRoot } from "./CanvasElementFormatPage";
 import { CanvasElementManager } from "../js/canvasElementManager/CanvasElementManager";
 import { kCanvasElementSelector } from "../toolbox/canvas/canvasElementConstants";
 import { getPageIFrame } from "../../utils/shared";
+import { getEditablePageBundleExports } from "../js/workspaceFrames";
 
 // Controls the CSS text-align value
 // Note: CSS text-align W3 standard does not specify "start" or "end", but Firefox/Chrome/Edge do support it.
@@ -587,6 +588,22 @@ export default class StyleEditor {
         if (styleSheet == null) {
             return null;
         }
+        if (create) {
+            // A caller asking us to create the rule means it is about to change it, and every
+            // change we make to these styles goes through the CSSOM -- setProperty, deleteRule,
+            // insertRule -- which mutates no DOM node. So the page watcher, which is a
+            // MutationObserver, cannot see it, and a formatting change that leaves the text alone
+            // would never be volunteered to C#: leaving the Edit tab or quitting would write the
+            // styles as they were. This is the one place every such change passes through.
+            //
+            // It is deliberately said BEFORE the change rather than after: the watcher waits a
+            // moment before reading the page, and the caller's edits are synchronous, so they are
+            // in by the time it looks. Saying so needlessly costs nothing -- an unchanged page
+            // produces no post.
+            // Through the page frame's exports rather than a direct import, because this class
+            // is used from the toolbox frame as well, and it is the PAGE frame that watches.
+            getEditablePageBundleExports()?.notePageContentMayHaveChanged();
+        }
 
         let ruleList: CSSRuleList = styleSheet.cssRules;
         if (ruleList == null) {

src/BloomBrowserUI/bookEdit/aiImageEditor/AGENTS.md

@@ -32,12 +32,12 @@ So:
   is there specifically so the two halves cannot disagree about which commit results belong to the
   page being edited.
 
-The overlay is in the top window because a page save reloads the page iframe, which would tear
-down an overlay hosted there — the same reason the image-gallery and copyright/license dialogs
-live up there (see the comments on those commands in `canvasControlRegistry.ts`). Note that this
-is *not* enough on its own: a save can also reload the whole workspace root, so C# waits for the
-page to come back before opening the overlay. `AiImageEditorApi.HandleSaveThenLaunch` explains
-that in full.
+The overlay is in the top window, like the image-gallery and copyright/license dialogs (see the
+comments on those commands in `canvasControlRegistry.ts`): plenty of operations replace the page
+iframe underneath it. Opening it first saves the page, because everything the editor is told about
+the book is read from the saved file; a save no longer reloads the page (BL-13502), so the editor
+opens as soon as the save returns, and does not open at all if the save did not happen.
+`AiImageEditorApi.HandleSaveThenLaunch` explains that in full.
 
 ## Tests
 

src/BloomBrowserUI/bookEdit/aiImageEditor/aiImageEditorOverlay.test.ts

@@ -9,10 +9,11 @@ import { beforeEach, describe, expect, test, vi } from "vitest";
 //  - The edit target. C# hands over the page id and file name of the image the user
 //    right-clicked (it survived a page save, which reloaded the page frame), and the overlay
 //    matches that against the book image list to fill the "Image to Edit" slot (BL-16682).
-//  - Saving after a commit. The current-page swaps only touched the LIVE DOM, so unless we
-//    save, a second commit in the same session would read its oldSrc from a saved page still
-//    showing the pre-edit image and match nothing. Because this overlay lives in the top
-//    window, we can save immediately: the page reload underneath leaves its controls alone.
+//  - NOT saving after a commit. A current-page swap lives in the live page DOM only, like an
+//    image pasted or chosen from the gallery, and is persisted the same way: by the normal page
+//    save when the user moves on (BL-16330). A retry from this still-open overlay would read a
+//    stale oldSrc, which the page frame handles by remembering what it already swapped rather
+//    than by saving here.
 
 const post = vi.fn();
 const postJson = vi.fn();
@@ -363,7 +364,7 @@ describe("aiImageEditorOverlay: the live page is NOT saved after a commit", () =
         };
         expect(ack.ok).toBe(true);
         expect(ack.error).toBeUndefined();
-        // Nothing landed on this page, so nothing to save.
+        // We never save from here at all -- see the note at the top of this file.
         expect(postThatMightNavigate).not.toHaveBeenCalled();
         expect(applyAiImageEditorReplacements).not.toHaveBeenCalled();
         postMessageToEditor.mockRestore();
@@ -389,7 +390,7 @@ describe("aiImageEditorOverlay: the live page is NOT saved after a commit", () =
         expect(ack.ok).toBe(false);
         expect(ack.error).toContain("not available");
         expect(ack.error).toContain("other pages were made");
-        // Nothing landed, so nothing to save.
+        // We never save from here at all -- see the note at the top of this file.
         expect(postThatMightNavigate).not.toHaveBeenCalled();
         postMessageToEditor.mockRestore();
     });

src/BloomBrowserUI/bookEdit/editablePage.ts

@@ -17,6 +17,11 @@ import {
 } from "./js/canvasElementManager/CanvasElementManager";
 import { kCanvasElementSelector } from "./toolbox/canvas/canvasElementConstants";
 import { renderDragActivityTabControl } from "./js/AbovePageControls";
+import {
+    getPageLoadId,
+    notePageContentMayHaveChanged,
+    startWatchingPageForSnapshots,
+} from "./js/pageSnapshot";
 
 function getPageId(): string {
     const page = document.querySelector(".bloom-page");
@@ -34,7 +39,12 @@ function getPageId(): string {
 // It is important that this does not get pulled into any other compiled bundle,
 // since it will generate errors when loaded into any page that does not have a .bloom-page.
 document.addEventListener("DOMContentLoaded", () => {
-    postString("editView/pageDomLoaded", getPageId());
+    // The load id goes with it: from here until the next page reports ready, C# accepts snapshots
+    // only from this load. See getPageLoadId().
+    postJson("editView/pageDomLoaded", {
+        pageId: getPageId(),
+        loadId: getPageLoadId(),
+    });
 });
 
 // This allows strong typing to be done for exported functions.
@@ -48,8 +58,13 @@ document.addEventListener("DOMContentLoaded", () => {
 // but I think it is unwise. It is so easy for an extra file to get imported into another bundle,
 // and then it will bring this along, with disastrous results.
 export interface IPageFrameExports {
-    requestPageContent(): void;
+    // The combined "body <SPLIT-DATA> userCss" string that a save needs, gathered without
+    // disturbing the live page.
+    getPageContentForSaveWhenReady(): Promise<string>;
     pageUnloading(): void;
+    // Say that the saved form of the page may have changed in a way the page watcher cannot see --
+    // the user's style definitions, which are changed through the CSSOM and mutate no DOM node.
+    notePageContentMayHaveChanged(): void;
     copySelection(): void;
     cutSelection(): void;
     pasteClipboard(): void;
@@ -110,12 +125,10 @@ export interface IPageFrameExports {
 }
 
 // This exports the functions that should be accessible from other IFrames or from C#.
-// For example, workspaceBundle.getEditablePageBundleExports().requestPageContent() can be called.
+// For example, workspaceBundle.getEditablePageBundleExports().pageUnloading() can be called.
 import {
-    getBodyContentForSavePage,
-    requestPageContent,
+    getPageContentForSaveWhenReady,
     captureContentForExternalProcessing,
-    userStylesheetContent,
     pageUnloading,
     topBarButtonClick,
     copySelection,
@@ -129,9 +142,11 @@ import {
     changeImageByElement,
     imageOperationCanUndo,
     imageOperationUndo,
+} from "./js/bloomEditing";
+import {
     addRequestPageContentDelay,
     removeRequestPageContentDelay,
-} from "./js/bloomEditing";
+} from "./js/pageContentDelays";
 import { showGamePromptDialog } from "./toolbox/games/GameTool";
 // Called from the AI Image Editor overlay in the top window, which owns the session but
 // cannot touch this page itself; see aiImageEditorPageCommands.ts and aiImageEditorOverlay.ts.
@@ -141,11 +156,10 @@ import type {
     IAiImageEditorCommitResult,
 } from "./aiImageEditor/aiImageEditorShared";
 export {
-    getBodyContentForSavePage,
-    requestPageContent,
+    getPageContentForSaveWhenReady,
     captureContentForExternalProcessing,
-    userStylesheetContent,
     pageUnloading,
+    notePageContentMayHaveChanged,
     topBarButtonClick,
     copySelection,
     cutSelection,
@@ -166,7 +180,7 @@ export {
     applyAiImageEditorReplacements,
 };
 import { origamiCanUndo, origamiUndo } from "./js/origami";
-import { postString } from "../utils/bloomApi";
+import { postJson } from "../utils/bloomApi";
 export { origamiCanUndo, origamiUndo };
 
 const styleSheets = [
@@ -383,6 +397,11 @@ $(document).ready(() => {
     // in the live editor, which never reads this flag.
     window.__bloomEditablePageReady = true;
 
+    // Start volunteering the page's content to C# whenever it changes and settles, so a save never
+    // has to ask for it and wait. See pageSnapshot.ts. Deliberately after bootstrap(), so the
+    // load-time fix-ups it applies are not themselves reported as the user's changes.
+    startWatchingPageForSnapshots(getPageContentForSaveWhenReady);
+
     // If the user clicks outside of the page thumbnail context menu, we want to close it.
     // Since it is currently a winforms menu, we do that by sending a message
     // back to c#-land. We have a similar listener in the pageThumbnailList itself.
@@ -400,11 +419,10 @@ export function SayHello() {
 // Legacy global exposure: mimic old webpack window["editablePageBundle"] contract used by other iframes / C#
 // NOTE: Keep this as a minimal curated surface: only expose functions intentionally callable cross-frame.
 interface EditablePageBundleApi {
-    requestPageContent: typeof requestPageContent;
     captureContentForExternalProcessing: typeof captureContentForExternalProcessing;
-    getBodyContentForSavePage: typeof getBodyContentForSavePage;
-    userStylesheetContent: typeof userStylesheetContent;
+    getPageContentForSaveWhenReady: typeof getPageContentForSaveWhenReady;
     pageUnloading: typeof pageUnloading;
+    notePageContentMayHaveChanged: typeof notePageContentMayHaveChanged;
     copySelection: typeof copySelection;
     cutSelection: typeof cutSelection;
     pasteClipboard: typeof pasteClipboard;
@@ -460,8 +478,8 @@ declare global {
         //      the finished page onto window.__bloomExternalPageContent.
         //   3. C# polls window.__bloomExternalPageContent until it is non-empty and reads it back.
         //
-        // Why globals + polling, rather than posting to the editView/pageContent API the way the live
-        // editor's requestPageContent() does:
+        // Why globals + polling, rather than posting to the editView/pageSnapshot API the way the
+        // live editor does:
         //   - That API feeds the live EditingModel; reusing it off-screen would corrupt the real
         //     editor's state. We want the same page-cleanup output, delivered out-of-band.
         //   - C#'s JS runner on this path (RunJavascriptWithStringResult_Sync_Dangerous) is
@@ -479,11 +497,10 @@ declare global {
 }
 
 window.editablePageBundle = {
-    requestPageContent,
     captureContentForExternalProcessing,
-    getBodyContentForSavePage,
-    userStylesheetContent,
+    getPageContentForSaveWhenReady,
     pageUnloading,
+    notePageContentMayHaveChanged,
     copySelection,
     cutSelection,
     pasteClipboard,

src/BloomBrowserUI/bookEdit/js/AbovePageControls.tsx

@@ -50,7 +50,7 @@ export function updateAbovePageControls(
 export function resetAbovePageControls(): void {
     currentState = defaultState;
     // Off-screen (e.g. process-book) there is no workspace frame, so this is a no-op there.
-    // (Reached via removeEditingDebris() in the shared extractAndStripPageContentForSave() save path.)
+    // (Reached via pageUnloading() in bloomEditing.ts.)
     tryGetWorkspaceBundleExports()?.setToolboxEnabled(true);
 
     const container = document.getElementsByClassName(