Save a page without reloading it; save from a snapshot the browser volunteers (BL-13502)

.claude/skills/run-bloom/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

.prettierignore

@@ -16,3 +16,8 @@ src/BloomBrowserUI/bookEdit/OverflowChecker/*Fixture.html
 # leave a dirty working tree, and prettier outright fails on the book .htm files. Leave the
 # whole tree alone so the fetched fixtures match what Bloom writes.
 output/testing-inputs/
+
+# The overflow checker's layout fixtures: the whitespace between their inline elements is
+# part of what OverflowChecker.browser.spec.ts measures, and prettier's reflow changes it
+# enough to turn its answers around.
+src/BloomBrowserUI/bookEdit/OverflowChecker/*Fixture.html

DistFiles/localization/en/BloomLowPriority.xlf

@@ -406,6 +406,11 @@
         <note>ID: ImageUtils.ShrinkingImagesFailed</note>
         <note>Shown as a toast when the one-time shrinking of an old book's oversized pictures fails. The book still works; the pictures are just left large, and Bloom retries later. "Bloom" is a product name and must not be translated.</note>
       </trans-unit>
+      <trans-unit id="Shell.SavingBeforeWindowsShutsDown" translate="no">
+        <source xml:lang="en">Bloom is saving your book</source>
+        <note>ID: Shell.SavingBeforeWindowsShutsDown</note>
+        <note>Shown by Windows itself, next to Bloom's name on the screen Windows displays when an application is slow to let it shut down, restart or log off. It appears only if Bloom takes more than a few seconds to finish saving. "Bloom" is a product name and must not be translated.</note>
+      </trans-unit>
       <trans-unit id="AiImageEditor.Credits.AccountCreditsUsed">
         <source xml:lang="en">{0} used of {1} account credits</source>
         <note>ID: AiImageEditor.Credits.AccountCreditsUsed</note>

docs/save-without-reload/HANDOFF.md

@@ -0,0 +1,133 @@
+# BL-13502 handoff: saving a page without reloading it
+
+For an agent continuing PR #8209 (branch `BL-13502-save-without-reload`, base `master`) on another
+machine, most likely to answer a human reviewer. Everything here is either not in the code or easy
+to miss there. Delete this folder before the PR merges.
+
+## Where things are
+
+- **The PR:** https://github.com/BloomBooks/BloomDesktop/pull/8209. Its description (between the
+  `preflight-narrative` markers) is the current summary of what the PR does, why, the risks, e2e
+  coverage and Notion cards. Keep it current if the PR changes.
+- **The card:** https://issues.bloomlibrary.org/youtrack/issue/BL-13502. It carries the tester's
+  notes (a comment marked `test-ideas`).
+- **The design:** `src/BloomExe/Edit/SavingWithoutReloading.md`. Read this first.
+- **The preflight report** (for John, with his decisions so far):
+  https://bloombooks.github.io/dev-process-artifacts/deciders/BloomDesktop-BL-13502-save-without-reload.html,
+  in the `BloomBooks/dev-process-artifacts` repo at
+  `deciders/BloomDesktop-BL-13502-save-without-reload.html`.
+- **The e2e plan** this PR's tests came from: a PR comment,
+  https://github.com/BloomBooks/BloomDesktop/pull/8209#issuecomment-6025653325.
+- **Devin:** https://devinreview.com/BloomBooks/BloomDesktop/pull/8209. Every finding has been
+  mirrored to the PR and answered; nothing is open.
+
+## Decisions John has made (don't reopen them without a reason)
+
+- **Every save uses the snapshot the browser volunteers.** No request carries the page. A
+  snapshot means the page is idle unless it carries `stillBusyWith`.
+- **No retries.** A failed snapshot post is reported to the user, once per page, and is not
+  retried: "Failure should be a crash."
+- **Only the `.bloom-page` element is gathered and sent.** C# wraps it in a trivial body. Nothing
+  reads the body element's attributes.
+- **A click, command or exit within about 25 ms of a keystroke** saves the snapshot from before
+  that keystroke. This is accepted: nobody can click that fast.
+- **The wait for a busy page** is a plain sleep with a two-second cap (`PageSnapshot.WaitUntilIdle`).
+  The log names the work that was still running if the cap is reached.
+- **Attribute order is ignored** when deciding whether a page changed
+  (`SafeXmlNode.GetXmlIgnoringAttributeOrder`). It is a generic XML helper, which is why it lives
+  in `SafeXmlNode`, not `HtmlDom`.
+- **Optional branding images that are missing get no "missing image" alt text**
+  (`isBrandingImage` in `bloomImages.ts`). That text was being saved into book-wide data.
+- **Table editing markup is stripped from the saved copy by Bloom's own copy of bloom-table's
+  list** (`removeTableEditingMarkupFromClone` in `tableEditing.ts`). bloom-table's
+  `removeTableEditingArtifacts` also ends Paint Format and Border Brush mode, which a save that runs
+  on every change must not do, and it doesn't export its markup-only part. A unit test compares the
+  two. If bloom-table ever exports that part, call it instead.
+- **Any request that makes C# save the page and then reload it** must first send the page:
+  `saveChangesAndRethinkPage` or `postAfterSendingSnapshot` in `pageSnapshot.ts`. A feature merged
+  from master that posts such a request directly will lose the last moments of typing. The
+  original-copyright sentence's unlock and relock needed this when master was merged on 10-09.
+- **The flaky test fix is in this PR.** `Xml_DoesNotProvide_ThreadSafety` was flaky on master;
+  John asked for the fix to go in here.
+- **Left alone on purpose:** bloom-player's `prepareActivity` leaves `touch-action: none` on
+  draggables, so playing a game page and then saving writes it into the book. It predates this
+  PR, and the tester's notes say not to report it.
+- **Comments say why the code is as it is now.** No history, except where old books can still
+  contain the older DOM.
+- **Fixed waits in e2e tests** are allowed only for a specific, known interval. The one in use is
+  `waitForPageToSettle`, which waits for the overflow checker's 1000 ms timer and then for the
+  page to have nothing left to send.
+
+## Running things on this branch
+
+- **C#:** build and test through `build/agent-dotnet.sh`, never bare `dotnet`. It builds into an
+  isolated folder.
+- **Bloom:** start it through the run-bloom skill. The launcher script is
+  `.claude/skills/run-bloom/launcherControl.mjs`, not `.github/skills/...`.
+- **E2E tests (`src/BloomE2E`)** run the `Bloom.exe` in `output/Debug`, not the isolated build. So:
+  1. Rebuild with `launcherControl.mjs --restart --wait-ready`.
+  2. Quit that Bloom with `launcherControl.mjs --quit-bloom`; it blocks a second Bloom. Vite stays
+     up.
+  3. Run `node build/get-testing-inputs.mjs` once.
+  4. Run the specs with `BLOOM_E2E_VITE_PORT=<the launcher's current vitePort>` and
+     `BLOOM_AUTOMATION_MONITOR=headless`:
+     `pnpm exec playwright test tests/<spec> --reporter=line`.
+
+  The fixture refuses a build older than any C# source.
+  After merging master, also run `pnpm run build:pageSizes` in `src/content`. It writes
+  `output/browser/pageSizesLookup.json`, which C# needs (about 30 C# tests fail without it). The
+  full front-end build would make it too, but agents must not run that. The commit hook reformats C# files
+  (csharpier), which can make the build look stale after a commit: rebuild, then rerun.
+- **The new specs:**
+  - `typing-survives-leaving-the-page`
+  - `saved-book-stays-clean`
+  - `change-layout-keeps-typing`
+  - `duplicate-page-many-times`
+
+  They have 16 tests, all passing.
+- **The older specs that touch this code:**
+  - `duplicate-page`
+  - `copy-page`
+  - `workspace-tabs`
+  - `toolbox-tools`
+  - `talking-book-paste`
+  - `rotate-and-flip-images`
+  - `capture-book-page`
+
+  All 28 of their tests pass.
+- **E2E tests that type and then leave the page** need `waitForBloomToHaveTyping` in between, as
+  a person's pause would provide. Master's specs, written before this PR, may not have it; on
+  10-09, `original-copyright-sentence.spec.ts` needed it.
+- **Quitting Bloom from a test:** `quitAndRestart` in the fixture posts `WM_CLOSE` to Bloom's
+  visible windows. In `--dont-disturb` mode the main window has a hidden owner, so `taskkill`
+  without `/F` and `CloseMainWindow` both miss it.
+- **CI:** the only PR workflow is `pr-automation`, which just starts Devin. On 2026-10-09 GitHub
+  started no run for the pushes that day. If Devin has no job for the head commit, start it by
+  loading the review page (see the `devin-review` skill).
+
+## Notion test cards
+
+Database "Test Case Runs", `38c4bb19-df12-8123-8bc8-e65b962cb12f`. Token: the
+`BLOOM_TESTCASE_NOTION` user environment variable, the secret of the internal integration
+`test-management-access`. Use the REST API directly (Notion-Version `2022-06-28`).
+
+The cards this PR automates are all set to Automation = `PR Pending`:
+
+- #659 Quit Without Losing Changes
+- #663 Clean Saved Book HTML
+- #841 Typing Survives Leaving the Page
+- #842 Looking at Pages Does Not Rewrite the Book
+- #843 Change Layout Keeps Typing
+- #844 Duplicate Page Many Times
+
+When the PR merges, set them to `Automated`. #366 notes that #843 covers part of it. #810's Many
+Times step moved to #844. Ask John before creating or changing cards.
+
+## Conventions for posting
+
+- **Attribution:** comments and replies posted under John's account start with a model tag, e.g.
+  `[Claude Opus 5.5 from John Thomson's machine]`.
+- **Devin:** never post an `@devin` mention.
+- **Human reviewers:** a fix you make at a reviewer's suggestion gets a short reply saying what
+  changed, then the thread is resolved. Disagreeing with a human goes to John first.
+- **Commits** end with the `Co-Authored-By` line for the model in use.

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,15 @@ export default class StyleEditor {
         if (styleSheet == null) {
             return null;
         }
+        if (create) {
+            // A caller asking us to create the rule is about to change it through the CSSOM, which
+            // the page watcher cannot see (see notePageContentMayHaveChanged). Every such change
+            // passes through here. Saying so before the change is fine: the watcher reads the page
+            // after a delay, and the caller's edits are synchronous.
+            // Through the page frame's exports rather than a direct import, because this class is
+            // also used from the toolbox frame, and it is the PAGE frame that watches.
+            getEditablePageBundleExports()?.notePageContentMayHaveChanged();
+        }
 
         let ruleList: CSSRuleList = styleSheet.cssRules;
         if (ruleList == null) {

src/BloomBrowserUI/bookEdit/aiImageEditor/AGENTS.md

@@ -38,12 +38,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.
 
 ## Analytics: Bloom forwards the AI Image Editor's events and knows nothing about them
 

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

@@ -9,9 +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).
-//  - NOT saving after a commit. A current-page swap lives only in the live page, like a pasted
-//    picture, and is saved by the normal page save when the user moves on. Saving at once
-//    would reload the page frame and discard the undo the swap registered (BL-16330).
+//  - 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();
@@ -376,7 +378,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();
@@ -402,7 +404,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/duplicateManyDialog.tsx

@@ -3,9 +3,11 @@ import { IBloomDialogEnvironmentParams } from "../react_components/BloomDialog/B
 import { useL10n } from "../react_components/l10nHooks";
 import { NumberChooserDialog } from "../react_components/numberChooserDialog";
 import { postData } from "../utils/bloomApi";
-import { WireUpForWinforms } from "../utils/WireUpWinform";
+import { ShowEditViewDialog } from "./workspaceRoot";
 
 export const DuplicateManyDialog: React.FunctionComponent<{
+    // The page the dialog was opened for. C# duplicates only if it is still the current page.
+    pageId: string;
     dialogEnvironment?: IBloomDialogEnvironmentParams;
 }> = (props) => {
     const title = useL10n(
@@ -26,6 +28,7 @@ export const DuplicateManyDialog: React.FunctionComponent<{
     const clickHandler = (value: number) => {
         postData("editView/duplicatePageMany", {
             numberOfTimes: value,
+            pageId: props.pageId,
         });
     };
     return (
@@ -40,4 +43,16 @@ export const DuplicateManyDialog: React.FunctionComponent<{
     );
 };
 
-WireUpForWinforms(DuplicateManyDialog);
+// Shown in the Edit tab, from the page list's context menu, so nothing reaches C# until the user
+// clicks OK. (A C# modal opened from the page list's request would hold the API lock.)
+export function showDuplicateManyDialog(pageId: string) {
+    ShowEditViewDialog(
+        <DuplicateManyDialog
+            pageId={pageId}
+            dialogEnvironment={{
+                initiallyOpen: true,
+                dialogFrameProvidedExternally: false,
+            }}
+        />,
+    );
+}