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

[JohnThomson]

Gathering a page's content for a save used to wreck the live page: the browser stripped the
editing markup out of the real DOM (detached the toolbox tool, removed origami layout mode and
text-box labels, killed the niceScroll bars, rewrote every bloom-editable with CKEditor's cleaned
data). What was left could be saved but not edited — which is exactly what the SavedAndStripped
state records, and why every save had to end by navigating somewhere.

This makes the gather non-destructive, and then starts spending that.

The core change

getBodyContentForSavePage() now clones the body and cleans the clone. Nothing at all happens
to the live page. Getting there needed:

• comicaljs 0.4.1 for Comical.exportSvgToCopiesOfParents — the non-destructive counterpart of
  stopEditing(), added for this in comical-js#120. (Moving off 0.3.106 also surfaced four
  pre-existing type errors: 0.3.106's declarations used a bare from "bubbleSpec" specifier, so
  BubbleSpec had silently been any.)
• Our own niceScroll cleanup (niceScrollCleanup.ts) for the clone, instead of asking the live
  scrollbars to remove themselves.
• One ITool.removeToolMarkup(pageOrClone) used two ways rather than a duplicated pair, with a
  console error if a detachFromPage override forgets its super call — because the symptom
  otherwise shows up much later, as tool markup saved into a book.
• CKEditor's data is read from the live editors and written into the clone.
• We no longer blur the active element on save, so the cursor stays put.

What that buys, so far

SaveThen now takes the current page's content when the caller can send it, and does the whole
save in one step instead of asking the browser and waiting for the answer on a separate API. Every
command the page list initiates does this: page click, duplicate, delete, paste, reorder, plus
Change Layout / import video / convert-to-derived-field.

Copy Page no longer reloads at all — it was reloading the very page it was copying, purely to
recover from the destructive save.

Removing the round trip also closes a real hole: while C# sat in SavePending waiting for the
browser, a second page click was silently discarded.

Safety

• Everything that gathers page content now waits on one gate, whenNoActiveDelays()
  (pageContentDelays.ts), so a save can't read a page mid-change. The synchronous gather is no
  longer exported anywhere, so there is no way around it.
• SaveThen is the only way in; the rule that only a Declined outcome may fall back lives in one
  place. Getting that wrong deletes a page twice — which it did, once, during development.

Testing

Driven against a real book in a running Bloom: every command saves the page's unsaved typing,
lands on a fully set-up page (CKEditor attached, canvas elements present), reaches disk, and never
fires editView/pageContent. Holding a delay stops a command from starting at all; a delay that
never clears lets it through after the 4s cap with a warning.

New unit tests: 21 for the state machine, 9 for the delay gate, 9 for niceScroll cleanup.

Note for reviewers

This is deliberately not merging into the current release. It is written to be cheap to merge
later — new behaviour in new files, and no reshaping of existing code just to add to it — because
what conflicts is a changed line, not an added one. src/BloomExe/Edit/SavingWithoutReloading.md
explains that, what has been converted, and what is left.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-13502

Devin review

---

This change is 

[JohnThomson]

[Claude Opus 5 from John Thomson's machine during preflight] Consulted Devin through three review rounds during this preflight, most recently up to e10b109.

It found three real bugs, all now fixed and each with its own resolved thread above:

• the reader tools' editing highlight was being saved into the book, because their cleanup only ever worked on the live page and the save now works from a clone;
• editView/savePageInPlace reported success even when the save was refused;
• removeToolboxMarkup() had lost every caller, so the open toolbox tool was no longer shut down when the user left a page.

It also raised a set of "is this intended?" flags. Four turned out to be worth acting on and are fixed and resolved (a missing disk write from the NoPage state, a brittle dependency on the reply's content type, and a page-list command that could vanish silently if the page frame navigated mid-wait). Four are deliberate design decisions or genuine trade-offs and are left open for John: the clone-cleanup ordering (which no single order gets right for both the reader tools and Talking Book), whether canvas-element alternates should still be conditional on canvas editing being on, and the click-time content snapshot.

Devin's own re-review of 38e8647 reported no unresolved bugs remaining. CI is green; CodeRabbit does not review this repo (auto_review is disabled in .coderabbit.yml).

[JohnThomson]

[Devin] Investigate: Dropping the 100ms delay relies on the API lock being released before the queued command runs

The thumbnail context-menu command is queued with BeginInvoke and runs after this handler returns. The comment says "we reply, the lock is released, and then the command runs" — Devin points out that ordering is not actually guaranteed.

The handler runs on the UI thread via Invoke from a server thread. When it returns, the server thread resumes and releases the API sync lock — but the UI thread is free to start pumping the queued command in the meantime. For "Duplicate Many Times" and "Choose Different Layout" that command opens a modal dialog whose content this same server has to serve, so in principle it could ask for content while the lock is still held.

Assessment: real, but the race is between a few microseconds (a thread-pool continuation calling Release) and at least a whole HTTP round trip (a dialog opening and fetching its content). It is very unlikely to lose. Note the old code's Task.Delay(100) made this safe by accident rather than by design — nobody wrote it down as the reason, which is exactly why it is worth recording now.

Left for a decision. The delay was removed at John's request to close a window in which typing could miss the snapshot, so putting it back is his call, not mine.

[JohnThomson]

[Devin] Investigate: Copy Page no longer selects the page it copied

Copy Page used to end by navigating to the page it copied. It no longer navigates at all — that is the deliberate change that makes copying stop reloading the page.

Devin's point is that those are only the same thing when the page you right-clicked IS the page you are on. A right-click does not select a page, so you can right-click a different thumbnail and choose Copy Page; the old behaviour would then have moved you to that page, and the new one leaves you where you are.

Assessment: arguably an improvement — a copy quietly moving you to another page is the more surprising behaviour of the two, and nothing about the copy needs you to be there. But it is a visible change to what the user sees, so it should be a choice rather than a side effect.

Left for a decision.

[JohnThomson]

[Claude Opus 5 from John Thomson's machine during preflight] Consulted Devin through four review rounds during this second preflight, most recently up to 5834032.

It found five more real defects, all fixed, each with a resolved thread above: an id that isn't a valid CSS identifier would have aborted the whole save and posted an error string instead of the page; the AI image editor stayed silent on two of the three ways its save can fail; a test kept a comment claiming an assertion it had lost; a failed save request was reported as though Bloom had refused; and the committed benchmark scripts only ran on my machine.

Two flags are left open for John, both about visible behaviour rather than defects: whether dropping the 100ms delay before a context-menu command is worth the small scheduling race it leaves (the delay was, undocumented, what made the ordering safe), and whether Copy Page should still move you to the page it copied when that isn't the page you are on.

Also worth recording from this run: preflight's own local review caught a deadlock risk I had introduced myself — dropping that deferral had left the command running inline in the API handler, while two of those commands open modal dialogs this same server must serve and the handler still holds the API lock.

Across both preflight runs Devin has found ten real defects in this work. None were caught by the tests or by driving the real UI. CI is green; CodeRabbit does not review this repo (auto_review is disabled in .coderabbit.yml).