Repository navigation
Conversation
…6893) "Update Book" -- both the Collections tab command and the automatic pass that runs before the AI image editor opens or after a page-size change -- showed a progress dialog that was mostly empty space. PR #8375 took the per-image log out of it but left the dialog alone: a tall window with the percent bar down at the bottom, three-quarters of it blank, and a three-sentence explanation set in the monospace log font. The explicit Update Book command came off worse than before, showing that title over a 400px-tall empty log box where the log had been. - Add SimpleProgressDialog: a title bar, a determinate bar with the percent beside it, and one sentence -- "Please wait while Bloom does some housekeeping on your book...". It shows no ordinary progress messages; a warning or error still colours the title bar and appears under the sentence, with Close and Report. It is a separate component from ProgressDialog rather than more switches on it: that one exists to show a running commentary the user is meant to read, its whole middle is a ProgressBox, and the options for hiding the log and shrinking the box had already piled up. - One instance of it lives in App.tsx, at the top level of Bloom's UI, and both callers open it over the websocket with props from the shared BookProcessor.MakeUpdateBookProgressProps, so the two cannot drift apart. App.tsx is the one document the tabs are rendered into and the dialog is a sibling of the active one, so it shows whichever tab you are on and sits outside the page iframe the Edit tab empties while the pass runs. - The automatic update no longer has a window of its own. It used a standalone WinForms ReactDialog, which is what required a separate JS bundle (an entry file, a vite entry and a ReactControl registration), a WinForms wrapper component, hard-coded window sizes, and code to resize a form that cannot size itself to its HTML. None of that is needed and all of it is gone. It did not need the WinForms modal in order to wait either: what the caller needs is to run its follow-up once the work has finished, which is what doWhenDialogCloses is for -- the mechanism Update Book already used. So EnsurePerPageFixupIfNeeded becomes EnsurePerPageFixupIfNeededThen, taking the follow-up as a continuation, and EditingModel's tail after the pass moves into ReturnToPageAfterFixup, reached through RunOffTheApiLock so it lands back on the UI thread and off the API lock. - Don't let one progress dialog pick up another's state. "progress" is a shared websocket channel heard by every dialog in the document, and each listener is alive from the moment it mounts, while the existing cleanup only ran for a dialog that had been open before. So a dialog that had never opened could accumulate somebody else's percent, errors and show-buttons and then open at the last job's percent with a red title bar and a live Close button, and pressing that Close told Bloom the job was over while the book was still being rewritten. SimpleProgressDialog now ignores events unless it is the dialog showing, and ProgressDialog clears all of its per-run state when it opens -- including the spinner, which is only ever switched off, by an event broadcast to every dialog. Both dialogs always had this; giving Update Book its own instance is what made it reachable. - Keep the dialog up for a problem that did not throw. Both callers return true when IWebSocketProgress.HaveProblemsBeenReported is set (it covers Warning, Error and Fatal alike), so the dialog waits for the user instead of closing the instant the work finishes and taking the unread message with it. - Don't lose the editor if the dialog cannot be opened. DoWorkWithProgressDialogAsync is "async Task" with no await in its own body, so a failure in the opening websocket send lands in the returned task rather than being thrown, and discarding that task would swallow it. Since the caller has already emptied the editor and the continuation is the only thing that puts a page back, that would have left a blank Edit tab with no dialog and no error. A faulted task is now logged and still runs the continuation. - The Report button sends every message of the run, not only the warnings and errors shown, so dropping the log did not cost the problem report its context. - Strings: BookProcessor.HousekeepingMessage is new in BloomMediumPriority.xlf, without translate="no" (319 of the 320 trans-units added to en/ on Version6.5 in the last 30 days omit it, and this string is now a whole dialog's content). BookProcessor.AutoUpdateExplanation is deleted rather than marked obsolete: no translation of it has ever existed -- the id appears in no language directory but en/ -- it was added on 2026-09-16 and never shipped, and nothing references it. AGENTS.md otherwise says never to delete a trans-unit, hence the evidence here. - Storybook stories for the ordinary case and the error case. One deliberate consequence: the old WinForms ShowDialog disabled every window in the application, so Bloom could not be closed during the automatic pass. An HTML backdrop covers everything Bloom draws but not the window's own chrome, so it can be now. Closing mid-pass abandons the run and Bloom offers to redo it next time. The Collections tab's Update Book has always been in this position. Verified against a running Bloom: both routes drive the one dialog; the automatic pass (page-size change on a 19-page book) runs, closes, and puts the editor back on the page it left; with a forced failure the title bar goes red, the dialog grows to fit the message, and pressing Close still restores the editor; and the "nothing to do" path restores it too. Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16893 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hatton, reviewing the dialog: "That kind of looks claustrophobic. I would add at least 10px to the bottom." BloomDialog leaves 10px under its content, which is fine for a dialog whose last row is a set of buttons but puts the text hard against the bottom edge when the content is one sentence. DialogMiddle now adds 14px of its own, so there is 24px below the sentence against the 20px above it. Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16893 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…16893) - The per-page pass reuses one off-screen browser for every page instead of a fresh renderer per page, and off-screen browsers skip Chromium's proxy lookup. Moon & Cap: about 9.9 s -> 5.7 s. - Each page records whether it has been brought up to date for the current browser maintenance level and page size (data-browser-maintenance-level / -layout, written by the editing JavaScript). The automatic pass only processes pages that are not yet marked, so pages edited in the Edit tab are skipped. This replaces the whole-book meta stamp. - A page-size change no longer runs the pass. The Publish tab runs it when a tool is chosen rather than when the tab opens, and holds the tool back until it is done; a second request while a pass runs waits for that pass instead of reading the book it is rewriting. - Progress bar: the whole-book update fills the first 5% so the bar never drops back; each run gets a fresh dialog, so it no longer slides back to 0 while closing; a finished run holds the full bar for 500 ms. - BloomBridge's process-book shows the same dialog, on its own websocket context, instead of ExternalBusyOverlay. The API replies and status are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…6893) Devin review: a finished dialog's cleanup, run when the next run replaces it, posted progress/closed a second time, which ends whatever BrowserProgressDialog job is current. The close now runs at most once. Choosing a Publish tool that opens at once also clears any tool still held back for a page update. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The per-page marks served the background-update experiment, which was dropped. Go back to the single whole-book record of the browser maintenance level and page size: the pass again processes every page when it runs. Kept: the reused off-screen browser, the progress split, running the pass when a Publish tool is chosen (not on a page-size change), the guard against two passes on one book, and BloomBridge's dialog. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…t-progress-dialog # Conflicts: # src/BloomBrowserUI/app/App.tsx # src/BloomBrowserUI/react_components/Progress/SimpleProgressDialog.tsx # src/BloomExe/Book/BookProcessor.cs # src/BloomExe/Edit/EditingModel.cs
…-16893) The book now carries a single <meta name="pageLayoutUpdateLevel">, in place of browserMaintenanceLevel and browserMaintenanceLayout. The per-page pass, and making a book from one of our own templates, set it to BookStorage.kPageLayoutUpdateLevel. A page size or orientation change, a size forced by the xmatter or branding while updating the book, and a Book Settings appearance change set it to 0, so the pages' recorded measurements get redone. A theme or margin change made the old record stale without it noticing. Saving lowers a level higher than ours to ours. The update removes the old two metas; the shipped Sample Shells carry the new one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…893) When the update ends with a problem the dialog stays up for the user to read it, but it still said "Please wait while Bloom does some housekeeping on your book...", which is no longer true. Once the job is over and has reported a problem, the dialog shows only the problem. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…' into BL-16893-e2e
page-layout-update.spec.ts covers: - A page size change or a theme change makes the next Publish tool run the update, and the one after it does not. - New books from Basic Book and from The Moon and the Cap need none. - A failed update is reported, the tool still opens after Close, the book stays marked as needing the update, and Bloom does not retry it. The failure comes from a new e2e/failPageUpdateAt hook, registered only under --e2e: when the pass reaches that page it throws, as a page whose capture failed would. New helpers: updateBookDialog.ts (watch for the update's dialog, arm a failure, read and close its problem report), bookSettings.ts (setBookTheme), showPublishDestination and isPublishDestinationShowing, readPageLayoutUpdateLevel, and makeBookFromSampleShell. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
[Medium risk] Adds end-to-end tests for page layout updates. The PR does not appear safe to merge until successive-page processing, appearance invalidation, and Publish completion ordering are addressed. Findings
SummaryThe PR adds end-to-end coverage for page-layout updates and a test-only failure hook, while also changing update scheduling, metadata, off-screen browser reuse, and progress UI.
Reviews (1) · Last reviewed commit: "E2E tests for when Bloom brings a book's..." |
hatton
commented
Sep 25, 2026
hatton
commented
Sep 25, 2026
hatton
commented
Sep 25, 2026
hatton
commented
Sep 25, 2026
…16893) The tool the user chose was recorded only when publish/switchingPublishMode replied. If the "pagesUpToDate" event arrived before that reply, the event found no tool waiting and was discarded, and the tool never opened. The choice is now recorded before the request goes out; the reply clears it when no update was needed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…' into BL-16893-e2e
The field is now _contactsOnlyBloomServer, and its comments say the point is where a browser's pages send HTTP requests, not which pages it shows: Bloom's browsers only show Bloom's own pages, but some of those pages (the AI Image Editor) send requests to other sites, which must go through the user's proxy. The off-screen browser's pages only contact Bloom's server. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…16893) Names and comments that still said "fix-up", "stamp", "due" or "browser maintenance level" now say "page layout update", matching the pageLayoutUpdateLevel meta: NeedsPageLayoutUpdate, UpdatePageLayoutIfNeededThen (BookProcessor and EditingModel, which was BringBookToCurrentBrowserLevelThen), RecordPageLayoutUpdateDone, the lock and underway set, and PageLayoutUpdateDecisionTests. A book whose update failed is no longer skipped until Bloom restarts. Most failures users see are transient, so the next Publish tool or AI image editor launch tries again. Publishing still goes ahead after a failure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bloom's editing code fixes up and measures each page when it lays the page out. Pages nobody opens never get that work, and a new page size, orientation, theme or margins leaves the recorded measurements wrong. So Bloom updates every page of a book at certain moments. - The book records a pageLayoutUpdateLevel meta. The update, and making a book from Bloom's own templates or Sample Shells, set it to the current level (BookStorage.kPageLayoutUpdateLevel). A page size or orientation change, a size forced by the branding or xmatter, and any Appearance change in Book Settings set it to 0. It replaces browserMaintenanceLevel and browserMaintenanceLayout, which the update removes. The shipped Sample Shells carry it. - The update runs when something needs the whole book: launching the AI image editor, or choosing a Publish tool, which opens once the update is done. Changing the page size no longer runs it. Update Book and BloomBridge always update every page. A second request while an update runs waits for it. - The update loads every page into one off-screen browser, which keeps the compiled editing code, and that browser skips the proxy lookup, since its pages only contact Bloom's own server. The Moon and the Cap: about 10 s to about 6 s. - The progress bar only moves forward and shows full for half a second before closing. BloomBridge's process-book uses the same dialog in place of ExternalBusyOverlay, with its replies and status unchanged. After a failure the dialog shows only the problem; Close opens the chosen tool, and the next attempt tries again. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…6893) The import makes pages and puts pictures on them without ever showing them in the Edit tab, so they lack what the editing code records when it lays a page out. It now sets pageLayoutUpdateLevel to 0, so the next Publish tool or AI image editor launch updates every page. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…preadsheet import change
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #8407. That PR targets Version6.5, where
src/BloomE2Edoes not exist, so these tests live on a branch made frommasterwith #8407's branch merged in. Merge #8407 first; once it has reached master, this PR's diff shrinks to the tests and the hook below.Bloom production code changes
src/BloomExe/web/controllers/E2eTestingApi.cs: newe2e/failPageUpdateAthook (registered only under--e2e). Its POST body is a 1-based page number.src/BloomExe/Book/BookProcessor.cs: when the page pass reaches that page, it throws, as a page whose content could not be captured does. The setting clears itself, so it affects one update only.Purpose
These tests protect when Bloom brings a book's pages up to date (BookProcessor's page pass), and when it doesn't:
Automates Notion test case 834 (Bloom Updates a Book's Pages When a Tool Needs Them): https://app.notion.com/p/Bloom-Updates-a-Book-s-Pages-When-a-Tool-Needs-Them-3e74bb19df128125bce8dc2d84c8b725
Steps
src/BloomE2E/tests/page-layout-update.spec.ts, five tests, each on a new book:rounded-border-ebook→ PDF & Print runs the update.How it verifies the result
pageLayoutUpdateLevelfrom the saved.htm. It expects "0" after a layout change and "1" after an update.New helpers
helpers/updateBookDialog.ts(new): watch for the update dialog, arm a failure, wait for the dialog's problem report, close it.helpers/bookSettings.ts(new):setBookTheme, which posts what the Book Settings dialog posts.helpers/publish.ts:showPublishDestinationandisPublishDestinationShowing.helpers/bookHtml.ts:readPageLayoutUpdateLevelandwaitForPageLayoutUpdateLevel.helpers/bookMaking.ts:makeBookFromSampleShell; the template lookup now takes the source collection's name.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16893
🤖 Generated with Claude Code
Devin review
This change is