Don't let a failed page save trap the user in Bloom (BL-16776)

[andrew-polk]

Problem

Switch a front cover to custom layout, add an image to it, and the book can reach a state where
the page can never be saved again. Every action that saves — clicking a page thumbnail, switching
to the Collections or Publish tab, even quitting — puts up "Bloom had trouble saving a page" and
then does nothing else. There is no way out of the program except Task Manager. Reported from
6.5.3007 with Little Zebra branding (BL-16776).

Cause

A branding html value can itself contain an <img> marked as the cover image. UpdateDomFromDataSet
collects every data-book node up front, then restores the branding element — which replaces
everything inside it, orphaning that img. When the orphan's turn comes, the update code dereferences
parents it no longer has and throws.

That alone would be one failed save. What makes it inescapable is separate: leaving the Edit tab and
shutting down both run their real work only after Book.Save() returns, so a save that throws
blocks each of them permanently.

Fix

• UpdateDomFromDataSet skips collected nodes that an earlier update has since detached from the
  document. This is the crash itself.
• Leaving the Edit tab now reports a failed Book.Save() and changes tabs anyway, instead of
  throwing out of the post-save action.
• Shutdown likewise reports and carries on, so quitting never depends on the save succeeding.
• EditingStateMachine.DoPostSaveAction calls the caller's failureAction when the post-save
  action throws, not only when applying the page content does — completing the recovery design
  that already existed for the other failure path.
• Regression test for the orphaned-img crash.

The wrong-image half of the report — the branding logo being designated as the cover image in the
first place, by normalizeCoverImageDesignation in bloomImages.ts — is not touched here, and has
no card of its own yet. BL-16780 (PR #8256) is the nearest work: it stops the same class of image
being picked as the book icon, in Book.GetCoverImagePathAndElt, and builds exactly the rule the
front end needs (an image counts only inside a bloom-imageContainer/bloom-canvas, plus a
branding/license/QR class exclusion). But it leaves the front-end designation code untouched, so it
does not prevent the corruption this PR recovers from.

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

Devin review

---

This change is 

[greptile-apps]

Greptile Summary

This PR prevents failed page saves from trapping users in Bloom while addressing the detached DOM node that caused the reported failure.

• Skips data-bound nodes detached by an earlier ancestor update.
• Allows tab changes and shutdown to continue after reporting save failures.
• Invokes the caller's recovery action when post-save work throws.
• Adds a regression test for branding HTML that orphans a cover image during synchronization.

Important Files Changed

Filename	Overview
src/BloomExe/Book/BookData.cs	Adds an attachment check that safely skips stale nodes orphaned by earlier DOM replacements during data synchronization.
src/BloomExe/Edit/EditingModel.cs	Reports full-save failures while allowing tab transitions and shutdown to complete instead of trapping the user.
src/BloomExe/Edit/EditingStateMachine.cs	Runs the registered failure callback when post-save work throws before restoring a navigable editing state.
src/BloomTests/Book/BookDataTests.cs	Adds a focused regression test covering a branding update that detaches a nested cover-image node.

_{Reviews (1): Last reviewed commit: "Report the tab-change save failure inste..." | Re-trigger Greptile}

[andrew-polk]

[Claude Opus 5] Consulted Devin on 2026-08-28 11:05 PDT up to commit 92a7fcce3da6ba375bb87ca0c4058c747f5dea34.

Re-review clean — no bugs and no Investigate flags. Four informational items, none mirrored: three of them just confirm the reasoning behind the change (that SafeXmlNode's == compares the wrapped node so the orphan walk is sound; that failureAction cannot now be invoked twice; that the PrepareElementsInPageOrDocument loop needs no orphan guard because it never walks to the document root), and the fourth flagged the missing guard on the first-pass loop, which is fixed in this same commit.

Greptile also reviewed the branch with no findings.