diff --git a/.claude/skills/run-bloom/SKILL.md b/.claude/skills/run-bloom/SKILL.md new file mode 100644 index 000000000000..fc61c0fe1b34 --- /dev/null +++ b/.claude/skills/run-bloom/SKILL.md @@ -0,0 +1,164 @@ +--- +name: run-bloom +description: Run, launch, screenshot, or drive the Bloom desktop app (Bloom.exe with embedded WebView2). Use when you need to start Bloom from this worktree, attach to a running Bloom, switch workspace tabs, take a screenshot, inspect DOM/console/network over CDP, or stop a Bloom instance. +--- + +# Run Bloom (desktop app) + +Bloom is a C#/WinForms shell hosting a React UI in WebView2. You drive it +programmatically: launch with `./go.sh`, discover the instance's HTTP/CDP +ports, then attach over CDP. The driver scripts live in +`.github/skills/bloom-automation/` (shared with Copilot agents) plus +`screenshotBloom.mjs` in this skill directory. All paths below are relative +to the repo root; all commands are bash unless noted. + +For deep detail (multi-instance rules, exe-backed Playwright tests, CDP +workflow), read `.github/skills/bloom-automation/SKILL.md`. This skill is the +verified quick path. + +## Prerequisites + +Dev machines already have: volta (provides node + yarn 1.22), .NET SDK 10, +WebView2 runtime. On a machine missing dependencies, run `./init.sh` +(fetches C# deps, yarn installs, initial `yarn build`) — documented but not +re-verified here; a failed C# build with CS0246 errors (missing +`PodcastUtilities` etc.) means `./init.sh` is needed. + +Never run `yarn build` while a watch/dev build is running (see AGENTS.md). +Never launch a previously built `Bloom.exe` directly — it can be stale. + +## Step 1: Is Bloom already running? + +```bash +node .github/skills/bloom-automation/bloomProcessStatus.mjs --running-bloom --json +``` + +Look at `runningBloomInstances` — each entry has `httpPort`, `cdpPort`, +`processId`, `detectedRepoRoot`, `vitePort`. This discovery is HTTP-based +(scans Bloom's standard port range, asks each instance +`/bloom/api/common/instanceInfo`) and keeps working even when WMI breaks +(see Gotchas). If an instance from **this** worktree is running, reuse it +and skip to Step 3. Don't kill another worktree's Bloom without asking. + +## Step 2: Launch from source + +Run in a **background** task (it is a long-lived launcher — never wait for +exit): + +```bash +./go.sh > /tmp/go-bloom.log 2>&1 # run_in_background +``` + +It starts Vite on a random port, waits for quiescence, then `dotnet watch +run` on `src/BloomExe`. Poll the log for the ready line (~2–4 min when the +C# build is warm): + +```bash +until grep -qE "Bloom ready\. HTTP|exited shortly after" /tmp/go-bloom.log; do sleep 2; done +grep -E "Bloom ready\. HTTP" /tmp/go-bloom.log | tail -1 +# => Bloom ready. HTTP 8092, CDP 8094, Bloom PID 51040. +``` + +The same info is on the machine-readable line +`BLOOM_AUTOMATION_READY {"processId":...,"httpPort":...,"cdpPort":...}`. +Use that HTTP port as the identity of your instance in every later command. +Multiple instances coexist; each takes the next port block (8089, 8092, …). + +Sanity check the instance: + +```bash +curl -s http://localhost:/bloom/api/common/instanceInfo +``` + +## Step 3: Drive it + +Switch workspace tabs (clicks the real tab over CDP, waits for the backend +to report it active): + +```bash +node .github/skills/bloom-automation/switchWorkspaceTab.mjs --http-port --tab edit --json +# --tab collection | edit | publish +``` + +Screenshot the embedded browser (lands in git-ignored `output/`): + +```bash +node .claude/skills/run-bloom/screenshotBloom.mjs --http-port --out output/screenshots/bloom.png --json +``` + +For arbitrary DOM/console/network work, attach Playwright (loaded from +`src/BloomBrowserUI/react_components/component-tester`) to +`http://localhost:` with `chromium.connectOverCDP` — see +`switchWorkspaceTab.mjs` and `screenshotBloom.mjs` as templates. On the Edit +tab, the page content lives inside the iframe named `page`; the top-level +document is shell UI. + +## Step 4: Stop the instance you started + +```bash +node .github/skills/bloom-automation/killBloomProcess.mjs --http-port --json +``` + +Check that `killedProcessIds` includes **the Bloom PID itself**, not just +its `dotnet` parents. In real runs here it was sometimes `[]` (WMI blind + +`taskkill` failing silently) and sometimes partial (parents killed, Bloom.exe +survived). For any PID still standing, fall back to PowerShell: + +```powershell +Stop-Process -Id -Force +``` + +Then stop the `./go.sh` background task itself (TaskStop or kill its shell). +This matters: after Bloom.exe dies, its `dotnet watch` chain stays alive +("Waiting for a file to change before restarting") and will **relaunch Bloom +on the next C# file edit** if left orphaned. + +## Human path + +`./go.sh` in a terminal; the Bloom window opens on the desktop; Ctrl+C shuts +the whole flow down. + +## Gotchas (all hit in real runs) + +- **WMI/wmic can go blind mid-session.** `bloomProcessStatus.mjs` (plain + mode) and `killBloomProcess.mjs` enumerate processes via `wmic`; on this + machine WMI stopped answering partway through a session — status reported + zero Bloom processes while one was demonstrably serving HTTP, and + `Get-CimInstance` hung for minutes. Trust the HTTP-based + `--running-bloom` discovery and `instanceInfo` over process enumeration. +- **`killBloomProcess.mjs` under-kills.** Observed both `killedProcessIds: + []` for a valid target and partial kills where the `dotnet watch` parents + died but Bloom.exe survived. Always verify the port went dark + (`instanceInfo` curl fails) and the Bloom PID is gone; `Stop-Process` any + survivors. +- **Orphaned `dotnet watch` chains relaunch Bloom.** If Bloom.exe is killed + but its watcher chain survives (e.g. someone kills Blooms from Task + Manager), the watchers sit at "Waiting for a file to change" and respawn + Bloom on the next C# edit. Check with `bloomProcessStatus.mjs --json` + (`watchProcesses`) and `Stop-Process` stale ones. +- **Never type `taskkill /PID ...` in Git Bash** — MSYS rewrites `/PID` to + `C:/Program Files/Git/PID`. Use the node helpers or PowerShell. +- **Grep for `Bloom ready\. HTTP`, not `Bloom ready\.`** — early in the log + watchBloomExe prints an instructional message that *quotes* the phrase + `'Bloom ready.'`, which matches the looser pattern long before launch + completes. +- **`dotnet watch` noise:** the launch log contains scary + `⚠ msbuild: [Failure] Package 'X' was restored using .NETFramework...` + lines. They are warnings; launch still succeeds. Don't grep the log for + bare `Failure`/`error` as a failure signal — wait for `Bloom ready.` or + `exited shortly after` instead. +- **`bloomProcessStatus.mjs` may print + `Assertion failed: !(handle->flags & UV_HANDLE_CLOSING)`** (libuv, on + exit, after the JSON is complete). Ignore it; the JSON on stdout is valid. +- **Agent shells may auto-background kill commands.** Capture the task + output file and read it; don't assume the inline result is the output. +- `body.className` of the top-level page is `developer` in dev builds; + page URL looks like `http://localhost:/bloom/C%3A/...Temp/bloomXXXX.htm`. + +## Tests + +Exe-backed Playwright suite (not re-run this session; see +`.github/skills/bloom-automation/SKILL.md` for detail): from +`src/BloomBrowserUI/react_components/component-tester`, +`BLOOM_HTTP_PORT= yarn playwright test --config playwright.bloom-exe.config.ts`. +TypeScript unit tests: `yarn test` in `src/BloomBrowserUI` (Vitest). diff --git a/.claude/skills/run-bloom/screenshotBloom.mjs b/.claude/skills/run-bloom/screenshotBloom.mjs new file mode 100644 index 000000000000..f768163631e7 --- /dev/null +++ b/.claude/skills/run-bloom/screenshotBloom.mjs @@ -0,0 +1,200 @@ +// Screenshot the embedded WebView2 of a running Bloom.exe over CDP. +// Companion to the helpers in .github/skills/bloom-automation/; reuses their +// instance discovery so it targets the exact Bloom that owns a given HTTP port. +// +// Usage: +// node .claude/skills/run-bloom/screenshotBloom.mjs (--running-bloom | --http-port ) [--out ] [--json] +import { createRequire } from "node:module"; +import { mkdirSync } from "node:fs"; +import path from "node:path"; +import { + fetchBloomInstanceInfo, + findRunningStandardBloomInstance, + getDefaultRepoRoot, + normalizeBloomInstanceInfo, + requireOptionValue, + requireTcpPortOption, + toLocalOrigin, +} from "../../../.github/skills/bloom-automation/bloomProcessCommon.mjs"; + +const parseArgs = () => { + const args = process.argv.slice(2); + const options = { + runningBloom: false, + httpPort: undefined, + out: undefined, + json: false, + }; + + for (let index = 0; index < args.length; index++) { + const arg = args[index]; + + if (arg === "--running-bloom") { + options.runningBloom = true; + continue; + } + + if (arg === "--http-port") { + options.httpPort = requireTcpPortOption( + "--http-port", + requireOptionValue(args, index, "--http-port"), + ); + index++; + continue; + } + + if (arg.startsWith("--http-port=")) { + options.httpPort = requireTcpPortOption( + "--http-port", + arg.slice("--http-port=".length), + ); + continue; + } + + if (arg === "--out") { + options.out = requireOptionValue(args, index, "--out"); + index++; + continue; + } + + if (arg.startsWith("--out=")) { + options.out = arg.slice("--out=".length); + continue; + } + + if (arg === "--json") { + options.json = true; + continue; + } + + if (arg === "--help") { + printHelp(); + process.exit(0); + } + } + + return options; +}; + +const printHelp = () => { + console.log( + "Usage: node .claude/skills/run-bloom/screenshotBloom.mjs (--running-bloom | --http-port ) [--out ] [--json]", + ); +}; + +const loadPlaywright = () => { + const repoRoot = getDefaultRepoRoot(); + const componentTesterDir = path.join( + repoRoot, + "src", + "BloomBrowserUI", + "react_components", + "component-tester", + ); + const requireFromComponentTester = createRequire( + path.join(componentTesterDir, "package.json"), + ); + + try { + return requireFromComponentTester("playwright"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error( + `Could not load Playwright from ${componentTesterDir}. Run 'yarn install' in src/BloomBrowserUI/react_components/component-tester if dependencies are missing. Original error: ${message}`, + ); + } +}; + +const resolveInstance = async (options) => { + if (options.httpPort) { + const response = await fetchBloomInstanceInfo(options.httpPort); + if (!response.reachable || !response.json) { + throw new Error( + `No Bloom instance reported common/instanceInfo on http://localhost:${options.httpPort}.`, + ); + } + + return normalizeBloomInstanceInfo(response.json, options.httpPort); + } + + if (options.runningBloom) { + const instance = await findRunningStandardBloomInstance(); + if (!instance) { + throw new Error( + "No running Bloom instance was found on Bloom's standard HTTP port range.", + ); + } + + return instance; + } + + throw new Error("Specify either --running-bloom or --http-port ."); +}; + +const getBloomPage = (browser) => { + const pages = browser.contexts().flatMap((context) => context.pages()); + return pages.find( + (candidate) => + candidate.url().includes("/bloom/") && + !candidate.url().startsWith("devtools://"), + ); +}; + +const main = async () => { + const options = parseArgs(); + const instance = await resolveInstance(options); + if (!instance.cdpPort) { + throw new Error( + "The selected Bloom instance did not report a CDP endpoint.", + ); + } + + const outPath = path.resolve( + getDefaultRepoRoot(), + options.out ?? path.join("output", "screenshots", "bloom.png"), + ); + mkdirSync(path.dirname(outPath), { recursive: true }); + + const { chromium } = loadPlaywright(); + const browser = await chromium.connectOverCDP( + toLocalOrigin(instance.cdpPort), + ); + + try { + const page = getBloomPage(browser); + if (!page) { + throw new Error( + `Could not find a Bloom WebView2 target on CDP port ${instance.cdpPort}.`, + ); + } + + await page.waitForLoadState("domcontentloaded"); + await page.screenshot({ path: outPath }); + + const result = { + instance: { + processId: instance.processId, + httpPort: instance.httpPort, + cdpPort: instance.cdpPort, + }, + pageUrl: page.url(), + screenshot: outPath, + }; + + if (options.json) { + console.log(JSON.stringify(result, null, 2)); + return; + } + + console.log(`Bloom HTTP port: ${result.instance.httpPort}`); + console.log(`Page URL: ${result.pageUrl}`); + console.log(`Screenshot: ${result.screenshot}`); + } finally { + await browser.close(); + } +}; + +main().catch((error) => { + console.error(error instanceof Error ? error.message : String(error)); + process.exit(1); +}); diff --git a/.github/skills/devin-review/SKILL.md b/.github/skills/devin-review/SKILL.md index c3b078d88884..0d629d7bc785 100644 --- a/.github/skills/devin-review/SKILL.md +++ b/.github/skills/devin-review/SKILL.md @@ -1,19 +1,20 @@ --- name: devin-review -description: Kick off a Devin AI code review for a GitHub PR, wait for it, then post unresolved Bugs and Investigate flags as GitHub PR comments. Devin does NOT post to GitHub automatically — this skill bridges that gap. +description: Kick off a Devin AI code review for a GitHub PR, wait for it, then post unresolved Bugs and Investigate flags as GitHub inline review-thread comments, and resolve the threads for findings Devin now considers fixed. Devin does NOT post to GitHub automatically — this skill bridges that gap. argument-hint: "PR URL or number, e.g. BloomBooks/BloomDesktop#7949" user-invocable: true --- - # Devin Review Skill ## When To Use + - When a new PR is created or a new commit pushed, as part of the bot-wait phase before human review. -- Invoked by `pr-ready-for-human` during its bot-wait stage. - When the user explicitly wants a Devin review run and reported. ## URLs + Given `owner/repo` and PR number (e.g. `BloomBooks/BloomDesktop` / `7949`): + - **Results page**: `https://app.devin.ai/review///pull/` Navigating to the results page both triggers a review (if none exists yet) and shows results once complete. The `devinreview.com///pull/` URL is an alias for the same page. @@ -25,9 +26,11 @@ The right sidebar of the review page has two tabs: **Info** and **Chat**. The **Info** tab contains: ### Bugs section + Header shows **"N Bug"** where N = count of *unresolved* bugs. Click to expand if collapsed. Each bug entry: + ``` {Title} Bug {file}:{line} ← unresolved @@ -35,20 +38,23 @@ Bug {file}:{line} ← unresolved Bug {file}:{line} • Resolved ← already fixed; skip ``` -→ **Post unresolved Bugs to GitHub** (Resolved ones are confirmed-fixed; no action needed.) +→ **Post unresolved Bugs to GitHub** as inline review-thread comments. **Resolved** bugs need cross-run reconciliation: if we posted the bug in a prior run, resolve its GitHub thread now; if we never posted it, no action (it was already fixed when Devin first saw it). See step 6. ### Flags section + Header shows **"N Flags"**. Click to expand — it is collapsed by default. Contains two sub-categories: **Investigate** items (post these): + ``` {Title} Investigate {file}:{line-range} ``` **Informational** items (skip these — these are the low-signal "comments"): + ``` {Title} Informational {file}:{line-range} @@ -57,6 +63,7 @@ Informational {file}:{line-range} → **Post Investigate flags to GitHub. Skip Informational flags.** ### Other sidebar fields + Checks, Reviewers, Assignees, Labels — these are metadata only, no action from this skill. ## Procedure @@ -95,11 +102,14 @@ chrome-devtools take_snapshot 2>/dev/null | grep -E "Bug|Investigate|Information Each finding appears as a button whose accessible name contains the title, type label (`Bug`, `Investigate`, `Informational`), and file:line. Resolved bugs additionally contain `• Resolved`. Collect: -- **Unresolved Bugs**: button text contains `Bug` but NOT `• Resolved` -- **Investigate flags**: button text contains `Investigate` -- **Skip**: buttons containing `• Resolved` (resolved bugs) or `Informational` + +- **Unresolved Bugs**: button text contains `Bug` but NOT `• Resolved` — to post (step 5) +- **Investigate flags**: button text contains `Investigate` — to post (step 5) +- **Resolved Bugs**: buttons containing `• Resolved` — record their titles; used to reconcile/resolve GitHub threads (step 6). Do NOT post these. +- **Skip entirely**: `Informational` items (low signal, no post, no reconcile) Expand the Flags section first if it is collapsed: + ```bash chrome-devtools evaluate_script "() => { const btn = [...document.querySelectorAll('button')].find(el => el.textContent.includes('Flags')); btn?.click(); return btn?.textContent?.trim(); }" sleep 2 @@ -130,67 +140,109 @@ Where `TITLE_PREFIX` is a distinctive prefix of the finding title (enough to be The extracted text will be: `{Title}\n\n{Full description paragraphs}` -### 5. Post Findings to GitHub +### 5. Post Findings to GitHub as Inline Review Threads + +Post each finding as an **inline review comment anchored to its diff line**, so it becomes a *resolvable* GitHub review thread. (Top-level PR comments have no "Resolve" affordance, so we can never close the loop on them — that is why we use review threads.) The "Post to GitHub" button on the Devin page is not functional — always post via `gh`. -The "Post to GitHub" button on the Devin page is not functional — always post via `gh` instead. +First gather the PR head commit and the existing Devin review threads. This one query serves both dedup (step 5) and resolution (step 6): it returns each thread's GraphQL id (to resolve), its first comment's REST id (to reply), the body (to match by title), and whether it is already resolved. -First check for existing Devin comments to avoid duplicates: ```bash -gh api repos///issues//comments --paginate --jq '.[].body' | grep "\[Devin\]" +HEAD_SHA=$(gh pr view --repo / --json headRefOid --jq .headRefOid) + +gh api graphql -f owner= -f repo= -F number= -f query=' +query($owner:String!,$repo:String!,$number:Int!){ + repository(owner:$owner,name:$repo){ + pullRequest(number:$number){ + reviewThreads(first:100){ nodes{ + id isResolved + comments(first:1){ nodes{ databaseId body } } + }} + } + } +}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] + | select(.comments.nodes[0].body | startswith("[Devin]")) + | {threadId:.id, isResolved, commentId:.comments.nodes[0].databaseId, body:.comments.nodes[0].body}' ``` -For each unresolved **Bug**: +A finding is **already posted** if one of those thread bodies contains the same finding title. Skip posting it again. + +**Post each unresolved Bug** (and each Investigate flag) that is not already posted. Use `Bug` or `Investigate` in the label: + ```bash -gh pr comment --repo / --body "$(cat <<'EOF' +# Single line ":": +gh api repos///pulls//comments \ + -f commit_id="$HEAD_SHA" \ + -f path="" \ + -F line= \ + -f side="RIGHT" \ + -f body="$(cat <<'EOF' [Devin] **Bug**: -`<file>:<line>` - -<Full description from the bug report if available via "See bug report" link> +<Full description> EOF )" ``` -For each **Investigate** flag: +For a **line range** `<start>-<end>`, add `-F start_line=<start> -f start_side="RIGHT"` and set `line=<end>`. + +**Fallback when the line is not in the diff** (the API returns HTTP 422 — common for Investigate flags on unchanged context lines): post a top-level comment instead so the finding is not lost, and tag it so step 6 can recognize it: + ```bash gh pr comment <number> --repo <owner>/<repo> --body "$(cat <<'EOF' -[Devin] **Investigate**: <Title> +[Devin] **Bug**: <Title> (`<file>:<line>` — outside diff, not resolvable as a thread) -`<file>:<line-range>` +<Full description> EOF )" ``` -Skip if a comment starting with `[Devin]` and containing the same title already exists. +Record which findings fell back — they cannot be natively resolved later (step 6 edits them instead). + +### 6. Reconcile Resolved Findings + +For each bug Devin now marks **• Resolved** (collected in step 3), match it by title against the Devin threads gathered in step 5: + +- **We posted it before and its thread is still unresolved** → reply to document why, then resolve the thread: + + ```bash + # Reply on the thread (needs the REST comment id from the query above) + gh api repos/<owner>/<repo>/pulls/<number>/comments \ + -F in_reply_to=<commentId> \ + -f body="[Devin] ✅ Devin now considers this fixed (as of \`$HEAD_SHA\`). Resolving." + + # Resolve the thread (needs the GraphQL thread id) + gh api graphql -f threadId=<threadId> -f query=' + mutation($threadId:ID!){ resolveReviewThread(input:{threadId:$threadId}){ thread{ id isResolved } } }' + ``` + +- **We posted it before only as a top-level fallback comment** (no thread) → it cannot be natively resolved; edit that comment to prepend a `✅ Resolved (<SHA>)` marker so a human sees it is closed. +- **We never posted it** → no action; it was fixed before we ever flagged it. +- **Its thread is already `isResolved: true`** → no action. + +Note: Investigate flags have no "Resolved" state in Devin — when Devin stops flagging one, it simply disappears from the list. Do **not** auto-resolve threads for vanished flags (no reliable signal); leave those for the human reviewer. -### 6. Report +### 7. Log that Devin was run — even if it found no issues + +Because it is quite expensive to consult Devin, it's important that we can avoid consulting it if we know that we have not committed anything since the last time we consulted it. For this reason, whenever a consultation with Devin is finished, add a comment to the PR: "Consulted Devin on (date time) up to commit (SHA)". Of course it is vital that we actually gave Devin sufficient time to do the check before we decide that it has no new findings. + +### 8. Report Return a summary: -- N unresolved Bugs found — N posted, N skipped (already posted), N resolved (skipped) + +- N unresolved Bugs found — N posted, N skipped (already posted), N fell back to top-level (line not in diff) +- N Resolved Bugs — N threads resolved, N fallback comments marked, N no-action (never posted / already resolved) - N Investigate flags found — N posted, N skipped - N Informational items found (not posted — low signal) - Whether any findings need developer attention before moving to human review -## Real Example (PR #7949) - -**Bugs (3 total, 1 unresolved):** -- ✅ Post: "Legacy ebook layout name normalization fails for mixed-case input" — `SizeAndOrientation.cs:80` -- ⏭ Skip: "buildSavePageContentString calls removeEditingDebris..." — `bloomEditing.ts:1322` — Resolved -- ⏭ Skip: "Overlay can get permanently stuck if exception occurs..." — `ExternalApi.cs:240` — Resolved - -**Flags (6 Investigate, several Informational):** -- ✅ Post: "Scale inconsistency in computeImageFitTopPercent..." — `autoFitImageOverTextSplits.ts:284` -- ✅ Post: "BringBookUpToDate may write to disk before per-page processing..." — `BookProcessor.cs:36` -- ✅ Post: "HandleProcessBookByPath could hit a stale FolderPath..." — `ExternalApi.cs:505` -- ✅ Post: "Tab change from ReloadCurrentBookDiscardingEdits may be asynchronous..." — `ExternalApi.cs:849` -- ✅ Post: "Device16x9 layouts get renamed to 'Ebook' in the user-facing display name" — `Layout.cs:169` -- ✅ Post: "Replace(\"ebook\", \"Ebook\") in ExtractPageSizeName is dead code" — `SizeAndOrientation.cs:76` -- ⏭ Skip: All Informational items (buildSavePageContentString DOM mutation, doWhenWorkspaceBundleLoaded, etc.) - ## Notes + - Devin does **not** post its findings to GitHub automatically — that is why this skill exists. -- A Resolved bug means Devin confirmed the PR already fixes what it found. No GitHub comment needed. +- Findings are posted as **inline review-thread comments** (anchored to the diff line), not top-level PR comments, specifically so they can be resolved later. +- A Resolved bug means Devin confirmed the PR already fixes what it found. If we **never posted** it, no GitHub action is needed. If we **did post** it in a prior run, resolve that thread now (step 6) so the comment we created doesn't linger looking unaddressed. +- Titles are the matching key between a Devin finding and the GitHub thread we posted for it, both for dedup (step 5) and resolution (step 6). Keep the `[Devin] **Bug**: <Title>` / `[Devin] **Investigate**: <Title>` format stable. - Informational items are observations, not action items. Skip them. - Use the `chrome-devtools` **CLI** (Bash commands) for all browser automation in this skill — not the MCP plugin (disabled; spawns zombie node processes) and not the Orca browser. - Always use `--isolatedContext "devin-noauth"` when opening Devin pages. Navigating while logged in consumes on-demand credits; the isolated context is unauthenticated but still shows all findings. - If Chrome DevTools CLI is unavailable, tell the user: "Please open `https://app.devin.ai/review/<owner>/<repo>/pull/<number>` in Chrome to check Devin's findings." + diff --git a/AGENTS.md b/AGENTS.md index 0d924498d4bc..774b024fe2cf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,9 @@ The vscode terminal often loses the first character sent from copilot agents. So # Running Bloom - Do not run an already-built `Bloom.exe` directly, because it may be stale and miss local code changes. -- Use a source-aware launcher that picks up the current repo state. Right now the default launcher is `./go.sh` at the repo root. +- Use a source-aware launcher that picks up the current repo state. Right now the default launcher is `./go.sh` at the repo root. If a build fails with errors like missing `PodcastUtilities`, `IDevice`, or other types/namespaces + that "could not be found" (CS0246) in files such as `src/BloomExe/Publish/BloomPub/usb/AndroidDeviceUsbConnection.cs`, the problem is probably that this worktree has not got its dependencies yet. Fix that with `./init.sh`. + - Do not launch Bloom with `dotnet run` or `node scripts/watchBloomExe.mjs` unless you are specifically working on the launcher scripts themselves or a better repo-supported source-aware launcher has been documented. If you create new files for temporary purposes (e.g. output or artifact or log files), be sure to clean them up when you're done and be careful not to accidentally commit them. diff --git a/src/BloomBrowserUI/bookEdit/OverflowChecker/OverflowChecker.ts b/src/BloomBrowserUI/bookEdit/OverflowChecker/OverflowChecker.ts index c07f9b04347a..035823747a91 100644 --- a/src/BloomBrowserUI/bookEdit/OverflowChecker/OverflowChecker.ts +++ b/src/BloomBrowserUI/bookEdit/OverflowChecker/OverflowChecker.ts @@ -11,6 +11,7 @@ import { addScrollbarsToPage, cleanupNiceScroll } from "bloom-player"; import { isInDragActivity } from "../toolbox/games/GameInfo"; import $ from "jquery"; import { kBloomButtonClass } from "../toolbox/canvas/canvasElementPageBridge"; +import { pageScrollsInsteadOfOverflowing } from "../js/scrollingLayouts"; interface qtipInterface extends JQuery { qtip(options: string): JQuery; @@ -505,12 +506,11 @@ export default class OverflowChecker { } const isButton = $editable.closest("." + kBloomButtonClass).length > 0; - if ( - $editable.parents("[class*=Device],[class*=Ebook]").length === - 0 || - isButton - ) { - // don't show an overflow warning if we have scrolling available (unless it's a button) + // don't show an overflow warning if we have scrolling available (unless it's a button) + const scrollingWillBeAvailable = + !!page[0] && + OverflowChecker.GetScrollInsteadOfOverflow(page[0]); + if (!scrollingWillBeAvailable || isButton) { theOneLocalizationManager .asyncGetText( "EditTab.Overflow", @@ -714,13 +714,7 @@ export default class OverflowChecker { } private static GetScrollInsteadOfOverflow(page: HTMLElement): boolean { - const $page = $(page); - return ( - $page.hasClass("Device16x9Portrait") || - $page.hasClass("Device16x9Landscape") || - $page.hasClass("Ebook2x3Portrait") || - $page.hasClass("Ebook7x5Landscape") - ); + return pageScrollsInsteadOfOverflowing(page); } // Make sure there are no boxes with class 'overflow' or 'thisOverflowingParent' on the page before removing // the page-level overflow marker 'pageOverflows', or add it if there are. diff --git a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts index 257e80cfef2c..d7daaf4823dc 100644 --- a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts +++ b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts @@ -1192,7 +1192,14 @@ export default class StyleEditor { // its nefarious work. The following block achieves this. // Enhance: this logic is roughly duplicated in toolbox.ts function doWhenCkEditorReadyCore. // There may be some way to refactor it into a common place, but I don't know where. - const editorInstances = (<any>window).CKEDITOR.instances; + // CKEditor is the rich-text editor. It can be entirely absent: off-screen book processing + // (external/process-book) strips it because nothing ever types into the page, yet a focusin + // can still reach this method during page setup. With no CKEditor there are no editor + // instances to coordinate with and no interactive style-editor gear worth attaching, so bail + // out instead of dereferencing undefined. + const ckeditor = (<any>window).CKEDITOR; + if (!ckeditor) return; + const editorInstances = ckeditor.instances; // (The instances property leads to an object in which each property is an instance of CkEditor) let gotOne = false; for (const property in editorInstances) { @@ -1208,9 +1215,7 @@ export default class StyleEditor { if (!gotOne) { // If any editable divs exist, call us again once the page gets set up with ckeditor. // no instance at all...if one is later created, get us invoked. - (<any>window).CKEDITOR.on("instanceReady", (e) => - this.AttachToBox(targetBox), - ); + ckeditor.on("instanceReady", (e) => this.AttachToBox(targetBox)); return; } const oldCog = document.getElementById("formatButton"); diff --git a/src/BloomBrowserUI/bookEdit/editablePage.ts b/src/BloomBrowserUI/bookEdit/editablePage.ts index 8fa3bcd4300e..aa8765d7a6f3 100644 --- a/src/BloomBrowserUI/bookEdit/editablePage.ts +++ b/src/BloomBrowserUI/bookEdit/editablePage.ts @@ -111,6 +111,7 @@ export interface IPageFrameExports { import { getBodyContentForSavePage, requestPageContent, + captureContentForExternalProcessing, userStylesheetContent, pageUnloading, topBarButtonClick, @@ -132,6 +133,7 @@ import { showGamePromptDialog } from "./toolbox/games/GameTool"; export { getBodyContentForSavePage, requestPageContent, + captureContentForExternalProcessing, userStylesheetContent, pageUnloading, topBarButtonClick, @@ -360,6 +362,15 @@ window["PasteImageCredits"] = () => { $(document).ready(() => { $("body").find("*[data-i18n]").localize(); bootstrap(); + // Step 1 of the off-screen page-capture handshake (see __bloomEditablePageReady in the + // `declare global` block below): bootstrap()/SetupElements() has now run. That applies the + // load-time DOM fix-ups (canvas-element layout, image sizing, etc.) — but note that some of them, + // notably image sizing, finish ASYNCHRONOUSLY after bootstrap() returns (they fetch image info and + // wait for images to load). Those register requestPageContent delays, and the capture step + // (captureContentForExternalProcessing) waits for those delays to clear, so setting this flag here + // just signals that bootstrap itself has run and it is safe to ask for the content. Harmless no-op + // in the live editor, which never reads this flag. + window.__bloomEditablePageReady = true; // 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 @@ -379,6 +390,7 @@ export function SayHello() { // 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; pageUnloading: typeof pageUnloading; @@ -421,11 +433,42 @@ interface EditablePageBundleApi { declare global { interface Window { editablePageBundle: EditablePageBundleApi; + // ── Off-screen page-capture handshake (C# BookProcessor ⇆ this bundle) ────────────────── + // The "process-book" feature (external/process-book API, used by BloomBridge to run + // finished books through Bloom's browser-only page fix-ups) re-saves every page of a book + // WITHOUT opening the live editor. For each page, C# loads it into a throwaway, off-screen + // WebView2 and runs this three-step handshake against the two globals below: + // + // 1. C# polls window.__bloomEditablePageReady until it is true. We set it (once, in + // $(document).ready below) the moment bootstrap()/SetupElements() returns. That kicks off + // the load-time DOM fix-ups (canvas-element layout, image sizing, ...); some of them finish + // asynchronously, which is exactly why step 2 still has to wait for in-flight work to settle. + // 2. C# calls editablePageBundle.captureContentForExternalProcessing() (defined in + // bloomEditing.ts). That waits for any in-flight async DOM work to settle, then stashes + // 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: + // - 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 + // synchronous, so it can't directly await the capture function's internal async settle. + // A plain window field it can poll is the simplest bridge. + // This looks fragile (two magic globals) but is well-contained: exactly one writer (the + // capture fn) and one reader (BookProcessor), and every page gets its own fresh disposable + // browser, so there is no stale-value or cross-page-bleed risk. + // + // Step 1's flag: set in $(document).ready below; read in BookProcessor.ProcessPage. + __bloomEditablePageReady?: boolean; + // Step 2/3's mailbox: the combined "body<SPLIT-DATA>userCss" string, or "ERROR: <message>". + __bloomExternalPageContent?: string; } } window.editablePageBundle = { requestPageContent, + captureContentForExternalProcessing, getBodyContentForSavePage, userStylesheetContent, pageUnloading, diff --git a/src/BloomBrowserUI/bookEdit/js/AbovePageControls.tsx b/src/BloomBrowserUI/bookEdit/js/AbovePageControls.tsx index 7481266139f2..d0d4a918199d 100644 --- a/src/BloomBrowserUI/bookEdit/js/AbovePageControls.tsx +++ b/src/BloomBrowserUI/bookEdit/js/AbovePageControls.tsx @@ -8,7 +8,7 @@ import { kIdForDragActivityTabControl, } from "../toolbox/games/DragActivityTabControl"; import { CogIcon } from "./CogIcon"; -import { getWorkspaceBundleExports } from "./workspaceFrames"; +import { tryGetWorkspaceBundleExports } from "./workspaceFrames"; interface IAbovePageControlsState { isDragGamePage: boolean; @@ -49,7 +49,9 @@ export function updateAbovePageControls( export function resetAbovePageControls(): void { currentState = defaultState; - getWorkspaceBundleExports().setToolboxEnabled(true); + // 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.) + tryGetWorkspaceBundleExports()?.setToolboxEnabled(true); const container = document.getElementsByClassName( "above-page-control-container", diff --git a/src/BloomBrowserUI/bookEdit/js/autoFitImageOverTextSplits.ts b/src/BloomBrowserUI/bookEdit/js/autoFitImageOverTextSplits.ts new file mode 100644 index 000000000000..8a3c3ec1c5c6 --- /dev/null +++ b/src/BloomBrowserUI/bookEdit/js/autoFitImageOverTextSplits.ts @@ -0,0 +1,333 @@ +import OverflowChecker from "../OverflowChecker/OverflowChecker"; +import { + kBackgroundImageClass, + kBloomCanvasSelector, + kCanvasElementSelector, +} from "../toolbox/canvas/canvasElementConstants"; +import { EditableDivUtils } from "./editableDivUtils"; + +// ── Auto-fit image/text origami splits (off-screen process-book only) ─────────────────────────── +// +// A page that is a single illustration in the first pane and a single text block in the second pane +// is usually saved at whatever ratio it was authored with (commonly 50/50). That often leaves a lot +// of empty space after the text while the image is much smaller than it could be. This grows the +// image pane (shrinking the text pane) as far as it can WITHOUT making the text overflow, but no +// further than the point where the image already fills the constraining page dimension (for +// top/bottom splits, the page width; for left/right splits, the page height). Growing past that just +// adds whitespace around the image. +// +// Governing rule: never leave the text overflowing. We don't estimate font/text sizes — we measure +// the real browser layout with OverflowChecker, and bias toward a hair more text room than the +// strict minimum. Wasted space is only a cosmetic ding; clipped text is a failure. +// +// Called by captureContentForExternalProcessing() (when process-book asks for it) so the grown +// split persists into the saved HTML. Mutates the live DOM; relies on the fresh disposable browser +// per page that the off-screen path uses. + +// Give the text a little more room than the exact overflow boundary (percent of split-pane height). +const kFitTextCushionPercent = 1.5; +// Never shrink the text pane below this percent of the split-pane height. +const kFitMinTextPercent = 5; + +// Auto-fit every qualifying image/text page in the document. Returns true if any page's split was +// changed (so the caller knows to re-fit the background images afterward). +export function fitImageOverTextSplits(): boolean { + const pages = Array.from( + document.querySelectorAll(".bloom-page"), + ) as HTMLElement[]; + let changedAny = false; + for (const page of pages) { + if (fitImageOverTextSplitOnPage(page)) changedAny = true; + } + return changedAny; +} + +type SplitOrientation = "horizontal" | "vertical"; + +interface SplitConfig { + orientation: SplitOrientation; + firstComponent: HTMLElement; + secondComponent: HTMLElement; + divider: HTMLElement; + firstInner: Element; + secondInner: Element; +} + +// Returns true if it changed this page's split (grew the image), false if it left the page alone. +function fitImageOverTextSplitOnPage(page: HTMLElement): boolean { + const marginBox = page.querySelector(".marginBox"); + if (!marginBox) return false; + + // We only handle the simple case: the marginBox's content is a single top-level two-pane split + // with no nested splits. + const splitPane = marginBox.querySelector( + ":scope > .split-pane.horizontal-percent, :scope > .split-pane.vertical-percent", + ) as HTMLElement | null; + if (!splitPane) return false; + if (splitPane.querySelector(".split-pane")) return false; // nested split: too complex, skip + + const splitConfig = getSplitConfig(splitPane); + if (!splitConfig) return false; + + // First pane must be a plain background image with no overlays; second pane must be text-only. + const firstCanvas = splitConfig.firstInner.querySelector( + kBloomCanvasSelector, + ) as HTMLElement | null; + const firstHasText = splitConfig.firstInner.querySelector( + ".bloom-translationGroup", + ); + // Overlays (canvas elements other than the background image) make this page out of scope. Our + // resizing math reasons only about the background image's aspect ratio; overlays don't scale with + // it predictably — a text bubble keeps its font size (changing line breaks / revealing or clipping + // content) and any bubble could end up extending past the resized image — so we leave such pages + // exactly as authored. Text overlays are technically also caught by firstHasText (a text bubble + // contains a .bloom-translationGroup), but this also excludes image/video/other overlays. + const firstHasOverlay = firstCanvas?.querySelector( + `${kCanvasElementSelector}:not(.${kBackgroundImageClass})`, + ); + const secondTextGroup = splitConfig.secondInner.querySelector( + ".bloom-translationGroup", + ) as HTMLElement | null; + const secondHasCanvas = + splitConfig.secondInner.querySelector(kBloomCanvasSelector); + if (!firstCanvas || firstHasText || firstHasOverlay) return false; + if (!secondTextGroup || secondHasCanvas) return false; + + const splitPaneRelevantSize = + splitConfig.orientation === "horizontal" + ? splitPane.offsetHeight + : splitPane.offsetWidth; + if (splitPaneRelevantSize <= 0) return false; + + // The stored split value is the second (text) pane's size as a percent of the split pane; the + // image pane gets the rest. Growing the image means shrinking this value. + const originalTextPercent = readSecondPanePercent(splitConfig); + + const setTextPercent = (percent: number) => { + const value = percent + "%"; + if (splitConfig.orientation === "horizontal") { + splitConfig.firstComponent.style.bottom = value; + splitConfig.divider.style.bottom = value; + splitConfig.secondComponent.style.height = value; + } else { + splitConfig.firstComponent.style.right = value; + splitConfig.divider.style.right = value; + splitConfig.secondComponent.style.width = value; + } + // Force a synchronous reflow so the measurements below see the new layout. + void splitPane.offsetHeight; + }; + + const textEditables = () => + Array.from( + secondTextGroup.querySelectorAll( + ".bloom-editable.bloom-visibility-code-on", + ), + ) as HTMLElement[]; + + // True if any visible text box has more text than fits in the (current) text pane. We check both + // kinds of overflow OverflowChecker knows about: the box overflowing itself (type 1) and the box + // overflowing its pane/ancestor (type 2, the usual one for auto-height origami text). + const textOverflows = (): boolean => + textEditables().some( + (e) => + OverflowChecker.IsOverflowingSelf(e) || + OverflowChecker.overflowingAncestor(e) !== null, + ); + + // Upper bound for the search: a text pane this tall is assumed to fit any text we'd auto-fit. + const hiBound = Math.max(originalTextPercent, 90); + + // If the text doesn't even fit when given most of the page, this isn't a page we can improve by + // growing the image (it's just over-full). Leave it exactly as we found it. + setTextPercent(hiBound); + if (textOverflows()) { + setTextPercent(originalTextPercent); + return false; + } + + // Binary-search the smallest text-pane percent at which the text still does not overflow. + let lo = kFitMinTextPercent; // largest known-overflowing value as we narrow (starts as a guess) + let hi = hiBound; // smallest known-fitting value + setTextPercent(lo); + if (textOverflows()) { + for (let i = 0; i < 12; i++) { + const mid = (lo + hi) / 2; + setTextPercent(mid); + if (textOverflows()) lo = mid; + else hi = mid; + } + } else { + hi = lo; // even a tiny text pane fits + } + const minTextPercent = hi; + + // Cap how far the image can grow: once it fills the page width, growing the image pane further + // just adds whitespace around the image, so don't shrink the text below that point. + const imageFitFirstPanePercent = computeImageFitFirstPanePercent( + splitPane, + firstCanvas, + splitConfig.orientation, + ); + + let finalTextPercent = minTextPercent + kFitTextCushionPercent; + if (imageFitFirstPanePercent !== undefined) { + const textFloorForImageFit = 100 - imageFitFirstPanePercent; + if (finalTextPercent < textFloorForImageFit) + finalTextPercent = textFloorForImageFit; + } + finalTextPercent = Math.min( + Math.max(finalTextPercent, kFitMinTextPercent), + hiBound, + ); + + // Only ever grow the image (shrink the text). If our computed split wouldn't enlarge the image, + // leave the page as authored. + if (finalTextPercent >= originalTextPercent) { + setTextPercent(originalTextPercent); + return false; + } + setTextPercent(finalTextPercent); + return true; +} + +function getSplitConfig(splitPane: HTMLElement): SplitConfig | undefined { + // A split pane is laid out either horizontally (panes stacked top/bottom) or vertically + // (panes side-by-side left/right). The two cases differ only in the marker class and the + // position classes of the two panes, so describe them in a table and share the lookup below. + const layouts: Array<{ + orientation: SplitOrientation; + percentClass: string; + firstPosition: string; + secondPosition: string; + }> = [ + { + orientation: "horizontal", + percentClass: "horizontal-percent", + firstPosition: "position-top", + secondPosition: "position-bottom", + }, + { + orientation: "vertical", + percentClass: "vertical-percent", + firstPosition: "position-left", + secondPosition: "position-right", + }, + ]; + const layout = layouts.find((l) => + splitPane.classList.contains(l.percentClass), + ); + if (!layout) { + return undefined; + } + + const firstComponent = splitPane.querySelector( + `:scope > .split-pane-component.${layout.firstPosition}`, + ) as HTMLElement | null; + const secondComponent = splitPane.querySelector( + `:scope > .split-pane-component.${layout.secondPosition}`, + ) as HTMLElement | null; + const divider = splitPane.querySelector( + ":scope > .split-pane-divider", + ) as HTMLElement | null; + const firstInner = firstComponent?.querySelector( + ":scope > .split-pane-component-inner", + ); + const secondInner = secondComponent?.querySelector( + ":scope > .split-pane-component-inner", + ); + if ( + !firstComponent || + !secondComponent || + !divider || + !firstInner || + !secondInner + ) { + return undefined; + } + return { + orientation: layout.orientation, + firstComponent, + secondComponent, + divider, + firstInner, + secondInner, + }; +} + +// Read the second (text) pane's size as a percent. The stylesheet defaults an unset split to 50%. +function readSecondPanePercent(splitConfig: SplitConfig): number { + const match = ( + splitConfig.secondComponent.getAttribute("style") || "" + ).match( + splitConfig.orientation === "horizontal" + ? /height:\s*([0-9.]+)%/ + : /width:\s*([0-9.]+)%/, + ); + return match ? parseFloat(match[1]) : 50; +} + +// The percent of the split-pane size the FIRST (image) pane needs so the image fills the limiting +// page dimension at its natural aspect ratio. Returns undefined if we can't determine it (e.g. image +// not yet loaded), in which case the caller simply skips the cap (the no-overflow guarantee still +// holds). Mirrors the aspect math in split-pane.ts getImagePercent(). +function computeImageFitFirstPanePercent( + splitPane: HTMLElement, + firstCanvas: HTMLElement, + orientation: SplitOrientation, +): number | undefined { + const aspectRatio = getImageAspectRatio(firstCanvas); + if (aspectRatio === undefined) return undefined; + const scale = EditableDivUtils.getPageScale() || 1; + const firstComponent = firstCanvas.closest( + ".split-pane-component", + ) as HTMLElement | null; + + if (orientation === "horizontal") { + const splitPaneHeight = splitPane.offsetHeight; + if (splitPaneHeight <= 0) return undefined; + const width = splitPane.offsetWidth; + const imageHeight = width / aspectRatio; + const extraHeight = firstComponent + ? (firstComponent.offsetHeight - firstCanvas.offsetHeight) / scale + : 0; + return ((imageHeight + extraHeight) * 100) / splitPaneHeight; + } + + const splitPaneWidth = splitPane.offsetWidth; + if (splitPaneWidth <= 0) return undefined; + const height = splitPane.offsetHeight; + const imageWidth = height * aspectRatio; + const extraWidth = firstComponent + ? (firstComponent.offsetWidth - firstCanvas.offsetWidth) / scale + : 0; + return ((imageWidth + extraWidth) * 100) / splitPaneWidth; +} + +// The DISPLAYED aspect ratio (width/height) of the image in this bloom-canvas, or undefined if +// unknown. +// We measure the rendered .bloom-backgroundImage canvas element rather than the <img>'s natural +// dimensions, because that box reflects any cropping the user applied — and it is the same source +// adjustBackgroundImageSizeToFit() (split-pane.ts getImagePercent()) uses, so our width cap agrees +// with how the image will actually be re-fit afterward. The background canvas element keeps its +// load-time size while we resize panes, so this aspect is stable across the binary search. +function getImageAspectRatio(bloomCanvas: HTMLElement): number | undefined { + const bg = bloomCanvas.getElementsByClassName( + "bloom-backgroundImage", + )[0] as HTMLElement | undefined; + if (bg && bg.clientWidth > 0 && bg.clientHeight > 0) { + return bg.clientWidth / bg.clientHeight; + } + // Fallback: the image's natural dimensions (ignores cropping, but better than nothing). These read + // as 0 until the image has loaded, and a missing/placeholder/corrupt image may never acquire a + // natural size; in those cases we return undefined and the caller simply skips the image-fit cap (the + // no-overflow guarantee from the binary search still holds). In the off-screen book processor the + // image-sizing delay (SetImageDisplaySizeIfCalledFor registers a requestPageContent delay) means + // images are normally loaded before we get here, so the .bloom-backgroundImage branch above usually + // wins anyway. + const img = bloomCanvas.querySelector("img") as HTMLImageElement | null; + if (img && img.naturalWidth > 0 && img.naturalHeight > 0) { + return img.naturalWidth / img.naturalHeight; + } + return undefined; +} diff --git a/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts b/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts index 12aef09d5d7a..7e9f489870d1 100644 --- a/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts +++ b/src/BloomBrowserUI/bookEdit/js/bloomEditing.ts @@ -77,6 +77,7 @@ import { EditableDivUtils } from "./editableDivUtils"; import { setupDragActivityTabControl } from "../toolbox/games/GameTool"; import { addScrollbarsToPage, cleanupNiceScroll } from "bloom-player"; import { setupBookLinkGrids } from "./linkGrid"; +import { fitImageOverTextSplits } from "./autoFitImageOverTextSplits"; import PlaceholderProvider from "./PlaceholderProvider"; import { initChoiceWidgetsForEditing } from "./simpleComprehensionQuiz"; import { handleUndo } from "../workspaceRoot"; @@ -1296,7 +1297,11 @@ function removeEditingDebris() { // Delay notification management for requestPageContent const activeDelays: string[] = []; -const kMaxWaitTimeMs = 2000; +// Upper bound (not a fixed wait) on how long we wait for in-flight async DOM work +// (image sizing, canvas-element layout, etc.) to finish before capturing anyway. The +// wait ends as soon as activeDelays empties, so simple pages are unaffected by this value; +// it only gives slower computers with complex pages more headroom before we give up. +const kMaxWaitTimeMs = 4000; let requestPageContentTimeout: number | null = null; // Add a delay notification that will prevent requestPageContent from running immediately. @@ -1369,25 +1374,36 @@ export function requestPageContent() { } } +// Run the load-time cleanup and return the page body + user stylesheet combined with the +// <SPLIT-DATA> delimiter that C# splits on. Shared by the live save path (requestPageContentInternal) +// and the off-screen capture path (captureContentForExternalProcessing) so the cleanup steps and the +// delimiter can't drift between them. +// +// DESTRUCTIVE READ: this mutates the live DOM as a side effect (removeToolboxMarkup(), +// removeEditingDebris(), and getBodyContentForSavePage() all strip classes, blur elements, turn off +// canvas-element editing, and do CKEditor cleanup) and does NOT restore it afterward. Both current +// callers tolerate this: the live editor re-navigates the page after saving, and the off-screen path +// uses a fresh disposable browser per page. Don't call this from a context where the page must stay +// live and editable afterward. +function extractAndStripPageContentForSave(): string { + // The toolbox is in a separate iframe, hence the call to getToolboxBundleExports(). (Off-screen, + // e.g. process-book, there is no toolbox iframe, so this is a no-op there.) + getToolboxBundleExports()?.removeToolboxMarkup(); + removeEditingDebris(); + const content = getBodyContentForSavePage(); + const userStylesheet = userStylesheetContent(); + // (We tossed up whether to use a JSON object instead of a delimiter, but combining two strings is + // simpler: HTML needs escaping to live in JSON, which we'd then have to undo in C#.) + return content + "<SPLIT-DATA>" + userStylesheet; +} + function requestPageContentInternal() { if (requestPageContentTimeout !== null) { clearTimeout(requestPageContentTimeout); } requestPageContentTimeout = null; try { - // The toolbox is in a separate iframe, hence the call to getToolboxBundleExports(). - getToolboxBundleExports()?.removeToolboxMarkup(); - removeEditingDebris(); // Enhance this makes a change when better it would only changed the - const content = getBodyContentForSavePage(); - const userStylesheet = userStylesheetContent(); - postString( - "editView/pageContent", - // We tossed up whether to use a JSON object here, but decided that it was simpler to just - // combine the two strings with a delimiter that we can split on in C#. - // For one thing, HTML requires some escaping to put in a JSON object, which would have - // to be done in Javascript, and then undone in C#. - content + "<SPLIT-DATA>" + userStylesheet, - ); + postString("editView/pageContent", extractAndStripPageContentForSave()); } catch (e) { postString( "editView/pageContent", @@ -1450,6 +1466,63 @@ export function getBodyContentForSavePage() { return result; } +// Used by the off-screen "process whole book" path (C# BookProcessor, driven by the +// external/process-book API). It gathers the same page content that requestPageContent() would save +// (via the shared extractAndStripPageContentForSave()), but instead of posting it to the editView/pageContent +// API (which feeds the LIVE EditingModel and would corrupt the live editor's state), it stashes the +// combined result on window.__bloomExternalPageContent for the C# caller to poll. Like +// requestPageContent(), it first waits for any in-flight async DOM work (activeDelays) to finish, up to +// kMaxWaitTimeMs, so browser-based measurements (image sizing, canvas-element layout, etc.) are complete +// before we capture the page. +export function captureContentForExternalProcessing( + fitImageTextSplits?: boolean, +): void { + window.__bloomExternalPageContent = undefined; + + // Optionally auto-fit simple single-image/single-text origami pages so the grown split persists + // into the saved HTML. This currently handles both image-above-text and image-left-of-text when + // the image is in the first pane. We do this UP FRONT, before the delay-wait below, for two reasons: + // - It must run on the fully settled, real browser layout (which it now is: bootstrap() and the + // load-time fix-ups have run before C# calls us). + // - Growing the image pane means the background image must be re-fit to the new pane size. That + // re-fit (adjustBackgroundImageSize) is async and registers a requestPageContent delay, so we + // kick it off here and let the waitForDelaysThenFinish loop below wait for it to settle before + // we capture. Otherwise we'd save the new split with the OLD (too-small) image, and the image + // would only get corrected later when a user opened the page in the Edit tab. + // Never throws out: a failure to fit must not block capturing/saving the page. + if (fitImageTextSplits) { + try { + const changedAny = fitImageOverTextSplits(); + if (changedAny) { + // Re-fit the background image(s) to the resized pane(s), exactly as the editor does + // after a programmatic splitter change (double-click "match previous page"). + theOneCanvasElementManager.adjustAfterOrigamiDoubleClick(); + } + } catch (e) { + console.error("fitImageOverTextSplits failed: ", e); + } + } + + const start = Date.now(); + const finish = () => { + try { + window.__bloomExternalPageContent = + extractAndStripPageContentForSave(); + } catch (e) { + window.__bloomExternalPageContent = + "ERROR: " + (e && e.message) + "\n" + (e && e.stack); + } + }; + const waitForDelaysThenFinish = () => { + if (activeDelays.length === 0 || Date.now() - start > kMaxWaitTimeMs) { + finish(); + } else { + setTimeout(waitForDelaysThenFinish, 50); + } + }; + waitForDelaysThenFinish(); +} + // Called from C# by a RunJavaScript() in EditingView.CleanHtmlAndCopyToPageDom via // workspaceBundle.getEditablePageBundleExports(). export const userStylesheetContent = () => { diff --git a/src/BloomBrowserUI/bookEdit/js/bloomImages.ts b/src/BloomBrowserUI/bookEdit/js/bloomImages.ts index f08e2c8def04..e49a89ed131d 100644 --- a/src/BloomBrowserUI/bookEdit/js/bloomImages.ts +++ b/src/BloomBrowserUI/bookEdit/js/bloomImages.ts @@ -1022,45 +1022,53 @@ function SetImageDisplaySizeIfCalledFor(container: JQuery, img: JQuery) { if (url.startsWith("/bloom/")) { return; } - getWithConfig( - "image/info", - { params: { image: GetRawImageUrl(img) } }, - (result) => { - const imageInfo: any = result.data; - const containerAspectRatio = - container.width() / Math.max(1, container.height()); - const imageAspectRatio = - imageInfo.width / Math.max(1, imageInfo.height); - - // image is skinnier than the container - if (imageAspectRatio < containerAspectRatio) { - $(img).css( - "width", - `${container.height() * imageAspectRatio}px`, - ); - $(img).css("height", "100%"); - } - // image is fatter than the container - else { - $(img).css( - "height", - `${ - imageInfo.height * - (container.width() / Math.max(1, imageInfo.width)) - }px`, - ); - $(img).css("width", "100%"); - } - // This isn't actually needed once we have the height and width - // here. However, when a new the book is previewed *before we've - // had a chance to set this stuff*, it will look awful unless we - // can put object-fit:contain on it. That won't make the border - // fit tightly, but at least the image won't be distorted. So we - // do set it that way in the stylesheet, and then overwrite that here once - // we have the height and width set. - $(img).css("object-fit", "cover"); - }, - ); + // Fetching the image info and applying the size is ASYNCHRONOUS, so this fix-up is NOT complete + // when SetupElements()/bootstrap() returns. Register a requestPageContent delay around it so any + // save/capture in flight waits for the sizing to finish before reading the DOM. In particular + // the off-screen book processor's captureContentForExternalProcessing() honors these delays, so + // it captures the sized page rather than the un-sized one. Mirrors adjustBackgroundImageSize(). + wrapWithRequestPageContentDelay(async () => { + const result = await getWithConfigAsync<IImageInfoResponse>( + "image/info", + { params: { image: GetRawImageUrl(img) } }, + ); + if (!result) { + return; + } + const imageInfo = result.data; + const containerAspectRatio = + container.width() / Math.max(1, container.height()); + const imageAspectRatio = + imageInfo.width / Math.max(1, imageInfo.height); + + // image is skinnier than the container + if (imageAspectRatio < containerAspectRatio) { + $(img).css( + "width", + `${container.height() * imageAspectRatio}px`, + ); + $(img).css("height", "100%"); + } + // image is fatter than the container + else { + $(img).css( + "height", + `${ + imageInfo.height * + (container.width() / Math.max(1, imageInfo.width)) + }px`, + ); + $(img).css("width", "100%"); + } + // This isn't actually needed once we have the height and width + // here. However, when a new the book is previewed *before we've + // had a chance to set this stuff*, it will look awful unless we + // can put object-fit:contain on it. That won't make the border + // fit tightly, but at least the image won't be distorted. So we + // do set it that way in the stylesheet, and then overwrite that here once + // we have the height and width set. + $(img).css("object-fit", "cover"); + }, "SetImageDisplaySizeIfCalledFor"); } } diff --git a/src/BloomBrowserUI/bookEdit/js/scrollingLayouts.ts b/src/BloomBrowserUI/bookEdit/js/scrollingLayouts.ts new file mode 100644 index 000000000000..26667ee6049b --- /dev/null +++ b/src/BloomBrowserUI/bookEdit/js/scrollingLayouts.ts @@ -0,0 +1,21 @@ +// The handful of page-size layouts meant to be viewed on a screen/device. When their content +// is too tall they show a scrollbar rather than reporting an overflow the way printed page sizes +// do. Keeping the list in one place lets the overflow checker (which works from a page element's +// classes) and the page-thumbnail list (which works from a layout-name string) agree on exactly +// which layouts these are. See BL-11949. +export const kScrollingLayouts = [ + "Device16x9Portrait", + "Device16x9Landscape", + "Ebook2x3Portrait", + "Ebook7x5Landscape", +]; + +// True if the named page-size layout scrolls instead of reporting overflow. +export function layoutScrollsInsteadOfOverflowing(layoutName: string): boolean { + return kScrollingLayouts.indexOf(layoutName) > -1; +} + +// True if the given page element's layout scrolls instead of reporting overflow. +export function pageScrollsInsteadOfOverflowing(page: HTMLElement): boolean { + return kScrollingLayouts.some((layout) => page.classList.contains(layout)); +} diff --git a/src/BloomBrowserUI/bookEdit/js/workspaceFrames.ts b/src/BloomBrowserUI/bookEdit/js/workspaceFrames.ts index 77566eba697f..2b05fbdefa9e 100644 --- a/src/BloomBrowserUI/bookEdit/js/workspaceFrames.ts +++ b/src/BloomBrowserUI/bookEdit/js/workspaceFrames.ts @@ -49,6 +49,14 @@ export function getWorkspaceBundleExports(): IWorkspaceExports { return workspaceBundle as IWorkspaceExports; } +// Like getWorkspaceBundleExports(), but returns null instead of throwing when there is no workspace +// bundle. Use this from code that can legitimately run with no surrounding workspace frame, e.g. the +// off-screen process-book context, where a page is loaded standalone with no parent workspace root. +export function tryGetWorkspaceBundleExports(): IWorkspaceExports | null { + const rootWindow = getRootWindow() as Window & { [key: string]: unknown }; + return (rootWindow["workspaceBundle"] as IWorkspaceExports) ?? null; +} + // Do this task when the workspace bundle is loaded. If it isn't loaded already, we set a timeout and do it when we can. // There is a similar doWhenToolboxLoaded in workspaceRoot.ts. export function doWhenWorkspaceBundleLoaded( diff --git a/src/BloomBrowserUI/bookEdit/pageThumbnailList/PageThumbnail.tsx b/src/BloomBrowserUI/bookEdit/pageThumbnailList/PageThumbnail.tsx index ed2b14a10f5d..a1a6bdc67f8b 100644 --- a/src/BloomBrowserUI/bookEdit/pageThumbnailList/PageThumbnail.tsx +++ b/src/BloomBrowserUI/bookEdit/pageThumbnailList/PageThumbnail.tsx @@ -4,6 +4,7 @@ import { useState, useEffect, useCallback } from "react"; import "../../lib/errorHandler"; import { get } from "../../utils/bloomApi"; import { IPage } from "./pageThumbnailList"; +import { layoutScrollsInsteadOfOverflowing } from "../js/scrollingLayouts"; // Global information across all PageThumbnails...see comments in requestPage() let lastPageRequestTime = 0; @@ -99,9 +100,9 @@ export const PageThumbnail: React.FunctionComponent<{ const reForOverflow = /^[^>]*class="[^"]*pageOverflows/; const overflowing = reForOverflow.test(content); // enhance: memo? - const scrollingWillBeAvailable = - props.pageLayout.indexOf("Device") > -1 || - props.pageLayout.indexOf("Ebook") > -1; + const scrollingWillBeAvailable = layoutScrollsInsteadOfOverflowing( + props.pageLayout, + ); useEffect(() => { if (Math.abs(Date.now() - lastPageRequestTime) > 5000) { diff --git a/src/BloomBrowserUI/collectionsTab/CollectionsTabPane.tsx b/src/BloomBrowserUI/collectionsTab/CollectionsTabPane.tsx index 12dd7f165a96..6747fccffde2 100644 --- a/src/BloomBrowserUI/collectionsTab/CollectionsTabPane.tsx +++ b/src/BloomBrowserUI/collectionsTab/CollectionsTabPane.tsx @@ -37,6 +37,7 @@ import { showMakeReaderTemplateBloomPackDialog, } from "../react_components/makeReaderTemplateBloomPackDialog"; import { AboutDialogLauncher } from "../react_components/aboutDialog"; +import { ExternalBusyOverlay } from "./ExternalBusyOverlay"; import { CollectionChooserDialog } from "../collection/CollectionChooserDialog"; const kResizerSize = 10; @@ -595,6 +596,7 @@ export const CollectionsTabPane: React.FunctionComponent = () => { <CollectionSettingsDialog /> <EmbeddedProgressDialog id="collectionTab" /> <MakeReaderTemplateBloomPackDialog /> + <ExternalBusyOverlay /> <CollectionChooserDialog open={collectionChooserOpen} onClose={() => setCollectionChooserOpen(false)} diff --git a/src/BloomBrowserUI/collectionsTab/ExternalBusyOverlay.tsx b/src/BloomBrowserUI/collectionsTab/ExternalBusyOverlay.tsx new file mode 100644 index 000000000000..728aa6830423 --- /dev/null +++ b/src/BloomBrowserUI/collectionsTab/ExternalBusyOverlay.tsx @@ -0,0 +1,93 @@ +import { css } from "@emotion/react"; +import * as React from "react"; +import { useState } from "react"; +import { Backdrop, CircularProgress, Paper } from "@mui/material"; +import { + useSubscribeToWebSocketForEvent, + useSubscribeToWebSocketForObject, +} from "../utils/WebSocketManager"; +import { kBloomBlue, kPanelBackground } from "../bloomMaterialUITheme"; +import { useL10n } from "../react_components/l10nHooks"; + +// A modal "busy" dialog shown while Bloom is doing a long-running external operation +// (currently external/process-book, which processes a book off-screen for ~20-30s). The C# side +// drives it over the "externalProcessing" websocket context: a "show" bundle (carrying a message) +// to raise it and a "hide" event to dismiss it. +// +// It is presented as a centered dialog box sitting on a FULLY OPAQUE backdrop, so the collection +// behind is hidden and cannot be clicked while Bloom is busy (the work runs on the UI thread, so +// interacting with the collection underneath would be a bad idea anyway). +// +// Why a websocket-driven curtain rather than a normal progress dialog: process-book runs +// synchronously on the UI thread (it creates off-screen WebView2 controls, which must be on that +// thread) and merely pumps the message loop with Application.DoEvents(). A BackgroundWorker-based +// dialog therefore doesn't fit, but because the UI thread keeps pumping, the main WebView2 keeps +// painting and this dialog's CSS spinner keeps animating (it runs in the renderer process) for +// the whole run. +export const ExternalBusyOverlay: React.FunctionComponent = () => { + const [open, setOpen] = useState(false); + const [message, setMessage] = useState<string | undefined>(); + + // Localized fallback shown if the C# side ever raises the overlay without supplying a message. + // Applied at render time (rather than captured in the websocket callback) so it reflects the + // localized value even though useL10n resolves asynchronously. + const defaultBusyMessage = useL10n( + "Bloom is busy, please wait…", + "Common.BloomIsBusy", + ); + + useSubscribeToWebSocketForObject<{ message?: string }>( + "externalProcessing", + "show", + (data) => { + setMessage(data.message); + setOpen(true); + }, + ); + useSubscribeToWebSocketForEvent("externalProcessing", "hide", () => { + setOpen(false); + }); + + return ( + <Backdrop + open={open} + css={css` + // Sit above everything else. This overlay is shown on the Collection tab, where the + // Edit-tab chrome (toolbox at 18000, origami at 60000, etc.) isn't present, but we use + // a value above those anyway so it stays the topmost layer regardless of context. + // (MUI modals default to 1300.) + z-index: 100000; + // Fully opaque (not the default translucent scrim) so the user can neither see nor + // click the collection behind while Bloom is busy. + background-color: ${kPanelBackground}; + `} + > + <Paper + elevation={8} + css={css` + display: flex; + flex-direction: column; + align-items: center; + padding: 30px 40px; + min-width: 280px; + max-width: 420px; + `} + > + <CircularProgress + css={css` + color: ${kBloomBlue}; + `} + /> + <div + css={css` + margin-top: 20px; + font-size: 16px; + text-align: center; + `} + > + {message ?? defaultBusyMessage} + </div> + </Paper> + </Backdrop> + ); +}; diff --git a/src/BloomExe/Book/BookProcessor.cs b/src/BloomExe/Book/BookProcessor.cs new file mode 100644 index 000000000000..969ca912f0cd --- /dev/null +++ b/src/BloomExe/Book/BookProcessor.cs @@ -0,0 +1,303 @@ +using System; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using System.Windows.Forms; +using Bloom.Api; +using Bloom.Edit; +using Bloom.ToPalaso; +using SIL.Progress; + +namespace Bloom.Book +{ + /// <summary> + /// Runs, off-screen, the same per-page "fix-up" that a user gets by opening a book in the Edit + /// tab and visiting every page, but without disturbing the live UI. It is driven by the + /// external/process-book API and used by BloomBridge to finish a freshly-generated + /// book whose raw HTML "isn't quite right" yet. + /// + /// Why a real browser is needed: when an editable page loads, the editing JavaScript + /// (bloomEditing.ts bootstrap/SetupElements) measures and mutates the DOM — image sizing, + /// canvas-element layout, font auto-fit, etc. We load each page into an off-screen WebView2, let + /// that initialization run, pull the resulting DOM back out, and save it through the normal + /// editing save path (Book.UpdateDomFromEditedPage), which strips editing markup and extracts + /// metadata. This mirrors the publish tab's off-screen page-check browser + /// (PublishHelper.BrowserForPageChecks). + /// </summary> + public static class BookProcessor + { + // Generous per-page limit; this is a background automation step, not interactive editing. + private const int kReadyTimeoutMs = 30000; + + /// <summary> + /// Bring the book structurally up to date (xmatter/layout migrations; this also ensures the + /// needed CSS file links are present, and on save the actual CSS files), then run the + /// per-page browser fix-up over every page and save the result to disk. Returns the number + /// of pages processed. + /// + /// All-or-nothing: a failure on any page (capture error or timeout) throws, the save at the + /// end is skipped, and nothing is persisted. The caller (external/process-book) surfaces this + /// as an error so BloomBridge can re-run, rather than leaving a half-processed book on disk. + /// + /// Must be called on the UI thread: it creates and pumps a WebView2 browser. + /// + /// When <paramref name="fitImageTextSplits"/> is true, simple two-pane origami pages with a + /// single illustration in the first pane and a single text block in the second pane have their + /// split auto-fit. This currently covers both image-above-text and image-left-of-text layouts. + /// The image pane is grown (and the text pane shrunk) as far as it can without making the text + /// overflow, but no further than the image filling the relevant page dimension. This uses the + /// real off-screen browser layout (no font/text estimation); see fitImageOverTextSplits() in + /// bloomEditing.ts. + /// </summary> + public static int ProcessBook(Book book, bool fitImageTextSplits = false) + { + Debug.Assert( + Program.RunningOnUiThread || Program.RunningUnitTests, + "BookProcessor.ProcessBook must run on the UI thread (it drives a WebView2)." + ); + + // 1. Structural "make it right" pass. Besides migrations, this ensures stylesheet links + // (and, when we Save below, the actual CSS files) that BloomBridge's raw HTML may + // be missing. See BookStorage.EnsureHasLinksToStylesheets. + // Log the book's identity so a developer can replay this exact run without BloomBridge, + // e.g. POST http://localhost:<port>/bloom/api/external/process-book {"id":"<id>"} + // (Bloom must be on the Collection tab.) The id is the book's bookInstanceId. + Log( + $"book id={book.ID} folder=\"{book.FolderPath}\" title=\"{book.NameBestForUserDisplay}\"" + ); + + // Creating the off-screen WebView2 controls below can yank the OS foreground onto Bloom, + // popping it in front of whatever the user (or the external tool that invoked + // process-book, e.g. BloomBridge) was looking at. This is a background processing step, so + // it has no business stealing focus. Remember who held the foreground up front and hand it + // back whenever a browser steals it (see RestoreForeground in the per-page loop and below). + // We capture this ONCE: every RestoreForeground call aims at this same original window, so + // if Bloom grabs the foreground again on a later page we still hand it back to where it + // started. (Re-capturing mid-batch would be wrong: by then Bloom itself often holds the + // foreground, so we'd "restore" to Bloom's own window.) + var priorForeground = ProcessExtra.GetForegroundWindow(); + + book.BringBookUpToDate(new NullProgress()); + + // 2. Per-page browser fix-up. + var pages = book.GetPages().Where(p => p != null).ToList(); + Log($"starting per-page fix-up of {pages.Count} pages (ckeditor stripped off-screen)"); + + var pageIndex = 0; + + // Create a FRESH WebView2 control per page, but share ONE CoreWebView2Environment across them. + // + // Fresh control per page: we must NOT reuse a single control. Each editing page is a full + // live-edit page (it opens the edit WebSocket channel and fires editView/* API calls on load); + // that residual state wedges the next top-level navigation in the same control, which hangs or + // crashes Bloom on the 2nd page. A fresh control gives each page a clean renderer, torn down on + // dispose, so every page behaves like the always-working first one. + // + // Shared environment: left to itself, each control would also create its OWN environment — a new + // browser process and a cold HTTP cache — costing ~300ms+ per page. Sharing one environment + // (one process, one user-data folder, one cache) across the batch removes that per-page cost and + // warms the cache so later pages navigate faster. Measured ~31s -> ~18s for a 22-page book + // (per-page init ~315ms -> ~100ms, nav ~940ms -> ~550ms after page 1). + WebView2Browser.BeginSharedEnvironmentBatch(); + try + { + foreach (var page in pages) + { + pageIndex++; + Browser browser = null; + try + { + browser = BrowserMaker.MakeBrowser(); + // Force the control's window handle into existence. The WebView2's CoreWebView2 + // initialization (kicked off unawaited in the browser constructor) can only complete + // once the control has a realized HWND; until it does, _readyToNavigate never becomes + // true and navigation just times out. HtmlThumbNailer does the same thing for its + // off-screen browser. (We never add this browser to a Form, so nothing else would + // create the handle for us.) + browser.CreateControl(); + + // If realizing the off-screen browser pulled Bloom to the front, put the + // previous window back so we keep processing quietly in the background. + RestoreForeground(priorForeground); + + ProcessOnePage(book, browser, page, fitImageTextSplits); + + Log($"page {pageIndex}/{pages.Count} [{page.Id}] done"); + } + finally + { + browser?.Dispose(); + } + } + } + finally + { + WebView2Browser.EndSharedEnvironmentBatch(); + // Final safety net: make sure we leave the foreground where we found it, even if a + // page threw partway through the batch. + RestoreForeground(priorForeground); + } + + // 3. One full save now that every page's in-memory DOM has been updated. + book.Save(); + + Log($"DONE: {pages.Count} pages"); + return pages.Count; + } + + /// <summary> + /// Hand the OS foreground back to <paramref name="priorForeground"/> if the off-screen browser + /// stole it for Bloom. No-op if we never knew the prior window or it still holds the foreground, + /// so we don't gratuitously flip windows around. Critically, we only restore when Bloom itself + /// currently holds the foreground: if some other application now holds it, the user has switched + /// away on purpose and we must not yank focus back out from under them. + /// </summary> + private static void RestoreForeground(IntPtr priorForeground) + { + if (priorForeground == IntPtr.Zero) + return; + var currentForeground = ProcessExtra.GetForegroundWindow(); + if (currentForeground == priorForeground) + return; // already where we want it + if (!ProcessExtra.IsWindowInCurrentProcess(currentForeground)) + return; // user has moved on to another app; leave their choice alone + ProcessExtra.SetForegroundWindow(priorForeground); + } + + // Write a process-book diagnostic line to BOTH the terminal (Bloom's stdout is piped to the + // `go` dev terminal) and the Bloom log file, so progress is visible live and after the fact. + private static void Log(string message) + { + var line = "[process-book] " + message; + Console.WriteLine(line); + SIL.Reporting.Logger.WriteEvent(line); + } + + private static void ProcessOnePage( + Book book, + Browser browser, + IPage page, + bool fitImageTextSplits + ) + { + var dom = book.GetEditableHtmlDomForPage(page); + // So the page's relative links (images, css) resolve against the book folder. + dom.BaseForRelativePaths = book.FolderPath; + + // Off-screen processing never types into the page, so CKEditor (the rich-text editor + // that AddJavaScriptForEditing injects for the live editor) is pure dead weight here: + // ~346KB of script to download/parse into each fresh renderer, plus an editor instance + // attached to every editable field during bootstrap(). None of it affects the load-time + // DOM fix-ups we capture. Strip the ckeditor <script> so it never loads; bootstrap()'s + // existing `typeof CKEDITOR === "undefined"` guard then skips all the attachment work. + var ckeditorScripts = dom.SafeSelectNodes("//script[contains(@src,'ckeditor')]"); + foreach (var script in ckeditorScripts) + script.ParentNode?.RemoveChild(script); + + // Frame == "Editing View is updating single displayed page": serves the page as-is. + // (Unlike JustCheckingPage, it does not swap videos for placeholder images.) + // + // We deliberately do NOT use NavigateAndWaitTillDone here. That waits for the WebView2 + // NavigationCompleted event (the window 'load' event), which is unreliable for a full + // editing page loaded off-screen: the editing bundle opens the edit WebSocket channel and + // fires editView/* API calls on load, and we have observed document.readyState getting + // stuck at "interactive" (load never firing) even though bootstrap()/SetupElements() has + // fully run and there are no pending sub-resources. Waiting for 'load' just burns the whole + // timeout. Instead we fire the navigation and then poll for __bloomEditablePageReady, which + // is the signal we actually care about (the load-time DOM fix-ups are in place). + // + // Navigate() blocks internally until the control is ready to navigate, so we pre-wait here. + // With the shared environment the heavy browser-process/cache warm-up is paid once for the + // batch, so after the first page this mostly waits on the control's handle/CoreWebView2 + // initialization. + var readyTimer = Stopwatch.StartNew(); + while (!browser.IsReadyToNavigate) + { + if (readyTimer.ElapsedMilliseconds >= kReadyTimeoutMs) + throw new ApplicationException( + $"process-book: timed out waiting for the browser to become ready to navigate on page {page.Id}." + ); + Application.DoEvents(); + Thread.Sleep(5); + } + + browser.Navigate(dom, source: InMemoryHtmlFileSource.Frame); + + // Wait until bootstrap()/SetupElements() has actually run (signaled by + // __bloomEditablePageReady), not merely until the bundle's exports exist, so the load-time + // DOM fix-ups are in place before we capture the page. + WaitForJavascriptResult( + browser, + "(window.__bloomEditablePageReady && window.editablePageBundle) ? 'ready' : ''", + "the editing bundle to initialize", + page + ); + + // Ask the bundle to gather the (now browser-processed) page content. It stashes the + // result on window for us to poll, rather than posting to the live editView API (which + // would feed the live EditingModel and corrupt the live editor's state). + // The boolean tells the bundle whether to auto-fit simple image/text origami splits before + // capturing (see fitImageOverTextSplits in bloomEditing.ts). + // Fire-and-forget: capture is asynchronous (it stashes onto window.__bloomExternalPageContent + // when it finishes) and we poll for that below, so there is no result to wait for here. The + // sync RunJavascriptWithStringResult_Sync_Dangerous would only block until the script's + // synchronous kickoff returned, which buys us nothing. + browser.RunJavascriptFireAndForget( + $"window.editablePageBundle.captureContentForExternalProcessing({(fitImageTextSplits ? "true" : "false")})" + ); + + var pageContent = WaitForJavascriptResult( + browser, + "window.__bloomExternalPageContent || ''", + "the page content to be captured", + page + ); + + if (pageContent.StartsWith("ERROR:", StringComparison.Ordinal)) + { + throw new ApplicationException( + $"process-book: failed to capture page {page.Id}: {pageContent}" + ); + } + + var editedDom = EditingModel.GetEditedPageDomFromBrowserContent(pageContent); + // Force a full update so shared/derived data (titles, metadata) is sucked in, matching + // what the live editor does when leaving a page that changed such data. We delay the + // actual write to disk until a single Book.Save() after all pages are processed. + book.UpdateDomFromEditedPage(editedDom, out _, needToDoFullSave: true); + } + + /// <summary> + /// Polls a javascript expression (which evaluates to a non-empty string when "ready") until + /// it is non-empty or we time out, returning the result. + /// + /// We deliberately keep this "poll a window global" approach for the off-screen processor + /// rather than the async editView/pageContent API callback the live editor uses: that callback + /// pattern needs the live EditingModel and edit WebSocket channel, which a throwaway off-screen + /// browser doesn't have, and the per-page loop here wants a deterministic in-line result. We are + /// aware RunJavascriptWithStringResult_Sync_Dangerous is the same family of call implicated in + /// past page-content deadlocks (BL-13120 etc.); moving this to an async/callback design is + /// deferred. That method already pumps the message loop while it waits for each script to + /// return, so we do NOT pump again between polls (just sleep briefly). + /// </summary> + private static string WaitForJavascriptResult( + Browser browser, + string script, + string whatWeAreWaitingFor, + IPage page + ) + { + var timer = Stopwatch.StartNew(); + while (timer.ElapsedMilliseconds < kReadyTimeoutMs) + { + var result = browser.RunJavascriptWithStringResult_Sync_Dangerous(script); + if (!string.IsNullOrEmpty(result)) + return result; + Thread.Sleep(20); + } + throw new ApplicationException( + $"process-book: timed out waiting for {whatWeAreWaitingFor} on page {page.Id}." + ); + } + } +} diff --git a/src/BloomExe/Book/BookStorage.cs b/src/BloomExe/Book/BookStorage.cs index 2d8e5daed25c..f8502fd8af6e 100644 --- a/src/BloomExe/Book/BookStorage.cs +++ b/src/BloomExe/Book/BookStorage.cs @@ -1847,8 +1847,18 @@ public void SetBookName(string name) } catch (Exception e) { - Logger.WriteEvent("Failed folder rename: " + e.Message); - Debug.Fail("(debug mode only): could not rename the folder"); + // The rename to the ideal (title-based) folder failed, most often because a + // different folder with that name already exists — e.g. another book with the same + // instanceId, which GetActualPathToSave deliberately reuses. This is recoverable: we + // simply keep the current folder (FolderPath is left unchanged below) and the book is + // already saved. We deliberately do NOT Debug.Fail here: that turned this handled, + // non-fatal condition into a hard crash in debug builds (e.g. external/process-book, + // which renames to the title on save and can hit an existing same-id/same-title + // folder). Just log it; release builds already behaved this way. + Logger.WriteEvent( + $"Failed to rename book folder from '{FolderPath}' to '{actualSavePath}'; " + + $"keeping the current folder. ({e.Message})" + ); } OnFolderPathChanged(); diff --git a/src/BloomExe/Book/SizeAndOrientation.cs b/src/BloomExe/Book/SizeAndOrientation.cs index 2125118061ae..b259de527a8a 100644 --- a/src/BloomExe/Book/SizeAndOrientation.cs +++ b/src/BloomExe/Book/SizeAndOrientation.cs @@ -73,6 +73,7 @@ private static string ExtractPageSizeName(string nameLower, int startOfOrientati name = name.Replace("folio", "Folio"); name = name.Replace("Uscomic", "USComic"); name = name.Replace("ebook", "Ebook"); + name = name.Replace("weaver", "Weaver"); return name; } @@ -80,7 +81,13 @@ private static string NormalizeLegacyPageSizeName(string pageSizeName) { // Handle books created in a future version of Bloom // that may use the planned for "Ebook16x9Landscape" and "Ebook9x16Portrait" names. - if (pageSizeName == "Ebook9x16" || pageSizeName == "Ebook16x9") + // Match case-insensitively as a defensive measure: a future version's class casing + // is not guaranteed (e.g. it might emit "EBook16x9" with a capital B), so compare + // without regard to case. + if ( + pageSizeName.Equals("Ebook9x16", StringComparison.OrdinalIgnoreCase) + || pageSizeName.Equals("Ebook16x9", StringComparison.OrdinalIgnoreCase) + ) return "Device16x9"; return pageSizeName; diff --git a/src/BloomExe/Collection/BookCollection.cs b/src/BloomExe/Collection/BookCollection.cs index d1776612edba..81a464d5ad47 100644 --- a/src/BloomExe/Collection/BookCollection.cs +++ b/src/BloomExe/Collection/BookCollection.cs @@ -448,13 +448,35 @@ private void AddBookInfo(string folderPath) // Mar 2025: I think this is no longer a problem, because the BookInfo constructor fully loads // AppearanceSettings. Not sure, so I'm leaving this code here, but I've made another exception, // because it's bad to use the selection BookInfo if it has the wrong SaveContext. - var bookInfo = + var reusableSelectionInfo = ( folderPath == _bookSelection.CurrentSelection?.FolderPath && _bookSelection.CurrentSelection.BookInfo.SaveContext == sc ) ? _bookSelection.CurrentSelection.BookInfo - : new BookInfo(folderPath, editable, sc); + : null; + // If an external tool (e.g. BloomBridge via external/update-book + + // process-book) has overwritten this folder on disk with a *different* book — one whose + // bookInstanceId no longer matches the selected book we have in memory — then reusing the + // selection's BookInfo would keep the stale id and hide the new book's identity from the + // collection. The on-disk id is what callers look the book up by, so a rescan that still + // reports the old id makes the new book unfindable (this is what made external/process-book + // fail with "could not find a book with id ..." on a re-import). Only reuse the selection's + // BookInfo when its id still matches what's on disk; otherwise read fresh. (A missing/unreadable + // meta.json leaves the id as-is, preserving the previous reuse behavior.) + // NOTE: BookMetaData.FromFolder is not a pure read — on a corrupt-but-recoverable meta.json + // it can restore from backup (delete/move) and can throw (IOException / FileException). Both + // are intentionally handled by the catch below, which degrades to an ErrorBookInfo just as a + // throwing `new BookInfo(...)` would, so don't "optimize" this assuming it only reads. + if ( + reusableSelectionInfo != null + && (BookMetaData.FromFolder(folderPath)?.Id ?? reusableSelectionInfo.Id) + != reusableSelectionInfo.Id + ) + { + reusableSelectionInfo = null; + } + var bookInfo = reusableSelectionInfo ?? new BookInfo(folderPath, editable, sc); bookInfo.ThumbnailLabel = bookInfo.GetBestDisplayTitle( _collectionSettings, _bookSelection.CurrentSelection diff --git a/src/BloomExe/CollectionTab/CollectionModel.cs b/src/BloomExe/CollectionTab/CollectionModel.cs index dd673f4b8fd4..20cca659ad63 100644 --- a/src/BloomExe/CollectionTab/CollectionModel.cs +++ b/src/BloomExe/CollectionTab/CollectionModel.cs @@ -85,6 +85,16 @@ LocalizationChangedEvent localizationChangedEvent public BookCollection CurrentEditableCollection => _currentEditableCollectionSelection.CurrentSelection; + /// <summary> + /// True if the open editable collection is a Team Collection (even if it is currently + /// disconnected or disabled). BloomBridge's external write endpoints (add/update/process-book) + /// refuse to operate on a Team Collection: doing so safely would require honoring checkout + /// state, preserving the TeamCollection.status file, and notifying the TC of renames, which + /// BloomBridge does not do. See ExternalApi. + /// </summary> + public bool IsEditableCollectionATeamCollection => + _tcManager?.CurrentCollectionEvenIfDisconnected != null; + /// <summary> /// The constructor of BookCommandsApi calls this to work around an Autofac circularity problem. /// </summary> @@ -189,6 +199,162 @@ public void DuplicateBook(Book.Book book) } } + /// <summary> + /// Copies a book folder from an arbitrary location on disk into the open editable collection, + /// reloads the collection, and selects the new book. Used by external automation (e.g. the + /// BloomBridge "keep this book" flow) to add a book it produced/processed elsewhere. + /// The book keeps its existing bookInstanceId. If a book with that same bookInstanceId is + /// already in the collection (e.g. this is a re-conversion of a book we imported before), the + /// existing folder is sent to the OS recycle bin and replaced by the incoming one. Otherwise, + /// only the destination folder name is made unique (and the main .htm renamed to match) if it + /// would collide with an unrelated existing book folder. + /// Returns the new Book, or null if it could not be located after the copy. + /// + /// Assumes the open collection is NOT a Team Collection: it strips the TeamCollection.status + /// file, recycles any same-id book without checking it out, and does not notify a TC of the + /// resulting add/rename. The only caller (external/add-book) refuses Team Collections up front + /// (see ExternalApi.RefuseIfTeamCollection), so that case never reaches here. + /// </summary> + public Book.Book AddBookFromFolder(string sourceBookFolderPath) + { + if ( + string.IsNullOrEmpty(sourceBookFolderPath) + || !Directory.Exists(sourceBookFolderPath) + ) + throw new ArgumentException( + "No book folder at " + sourceBookFolderPath, + nameof(sourceBookFolderPath) + ); + // Trim any trailing separator so GetFileName/GetDirectoryName behave. + sourceBookFolderPath = sourceBookFolderPath.TrimEnd( + Path.DirectorySeparatorChar, + Path.AltDirectorySeparatorChar + ); + if (!RobustFile.Exists(Path.Combine(sourceBookFolderPath, "meta.json"))) + throw new ArgumentException( + "Folder is not a Bloom book (no meta.json): " + sourceBookFolderPath, + nameof(sourceBookFolderPath) + ); + + var collectionDir = TheOneEditableCollection.PathToDirectory; + + // It's an error to import from a source folder that is itself inside this collection: + // that would be copying a book onto itself. (This is a different situation from the + // same-bookInstanceId replacement handled below, where the source lives OUTSIDE the + // collection but an existing book in the collection shares its id and/or folder name.) + if ( + string.Equals( + Path.GetDirectoryName(sourceBookFolderPath), + collectionDir.TrimEnd( + Path.DirectorySeparatorChar, + Path.AltDirectorySeparatorChar + ), + StringComparison.OrdinalIgnoreCase + ) + ) + throw new ArgumentException( + "Book is already in the collection: " + sourceBookFolderPath, + nameof(sourceBookFolderPath) + ); + + // If this is a re-import of a book we already have (same bookInstanceId), send the existing + // copy to the recycle bin so the incoming one replaces it rather than piling up beside it + // under a uniquified name. We match on id (not folder name) so a retitled re-conversion still + // replaces its predecessor. The recycle is recoverable from the OS trash if it was a mistake. + var incomingId = TryGetBookInstanceId(sourceBookFolderPath); + if (!string.IsNullOrEmpty(incomingId)) + { + var existing = TheOneEditableCollection + .GetBookInfos() + .Where(info => info.Id == incomingId) + .ToList(); + foreach (var info in existing) + { + // If the book we're about to replace is the current selection, drop the selection + // first so we don't leave it pointing at a folder that's headed for the trash. + if (_bookSelection.CurrentSelection?.FolderPath == info.FolderPath) + _bookSelection.SelectBook(null); + // Abort the import if we can't actually remove the existing copy. Continuing would + // leave two books sharing incomingId, which breaks later id-based targeting (and is + // the opposite of the intended replace). Recycle has already notified the user of the + // underlying failure; the throw surfaces it to the external caller too. + if (!ConfirmRecycleDialog.Recycle(info.FolderPath)) + throw new IOException( + $"Could not recycle the existing book at \"{info.FolderPath}\" to replace it; " + + "aborting import to avoid creating a duplicate book id." + ); + TheOneEditableCollection.HandleBookDeletedFromCollection(info.FolderPath); + } + } + + var baseName = Path.GetFileName(sourceBookFolderPath); + // Keep the clean folder name when it's free; only fall back to a unique (TC-safe) name on + // collision, as DuplicateBook does. (After the recycle above, a re-import normally reclaims + // its original clean name.) + var newBookName = Directory.Exists(Path.Combine(collectionDir, baseName)) + ? BookStorage.GetUniqueBookFolderName(collectionDir, baseName) + : baseName; + var newBookDir = Path.Combine(collectionDir, newBookName); + + // Copy everything except the throwaway/derived files BookStorage.Duplicate also skips. + BookStorage.CopyDirectory( + sourceBookFolderPath, + newBookDir, + new[] { ".bak", ".bloombookorder", ".pdf", ".map" } + ); + + // If we had to uniquify the folder name, the main .htm inside still has the old name. Rename + // it to match the new folder so Bloom finds it cleanly (mirrors BookStorage.Duplicate). + if (!string.Equals(newBookName, baseName, StringComparison.Ordinal)) + { + var oldHtm = Path.Combine(newBookDir, baseName + ".htm"); + if (!RobustFile.Exists(oldHtm)) + // Fall back to whatever single book htm we copied. + oldHtm = Directory + .GetFiles(newBookDir, "*.htm") + .FirstOrDefault(p => !Path.GetFileName(p).StartsWith(".")); + if (oldHtm != null && RobustFile.Exists(oldHtm)) + RobustFile.Move(oldHtm, Path.Combine(newBookDir, newBookName + ".htm")); + } + + // Strip any Team-Collection / local-only status copied from the source, so Bloom treats + // this as a normal local book. + BookStorage.RemoveLocalOnlyFiles(newBookDir); + + ReloadEditableCollection(); + + var newInfo = TheOneEditableCollection + .GetBookInfos() + .FirstOrDefault(info => info.FolderPath == newBookDir); + if (newInfo == null) + return null; + + var newBook = GetBookFromBookInfo(newInfo); + SelectBook(newBook); + BookHistory.AddEvent( + newBook, + BookHistoryEventType.Created, + $"Imported book from \"{sourceBookFolderPath}\"" + ); + return newBook; + } + + /// <summary> + /// Reads just the bookInstanceId from a book folder's meta.json without the side effects of + /// constructing a full BookInfo. Returns null if the folder has no readable metadata. + /// </summary> + private static string TryGetBookInstanceId(string bookFolderPath) + { + try + { + return BookMetaData.FromFolder(bookFolderPath)?.Id; + } + catch (Exception) + { + return null; + } + } + public void moveBookIntoThisCollection(Book.Book origBook, BookCollection origCollection) { var possibleTCFilePath = TeamCollectionManager.GetTcLinkPathFromLcPath( diff --git a/src/BloomExe/Edit/EditingModel.cs b/src/BloomExe/Edit/EditingModel.cs index 7f18147a81eb..a20fbb7e48cb 100644 --- a/src/BloomExe/Edit/EditingModel.cs +++ b/src/BloomExe/Edit/EditingModel.cs @@ -41,6 +41,12 @@ public class EditingModel private readonly ITemplateFinder _sourceCollectionsList; private bool _havePageToSave; + // Set by ReloadCurrentBookDiscardingEdits when an external tool has overwritten the current + // book on disk while the Edit tab is live. It tells the leaving-Edit-tab logic in + // OnTabAboutToChange to reload the book from disk instead of saving, so the user's unsaved + // page is discarded in favor of the new on-disk content rather than clobbering it. + private bool _reloadFromDiskOnLeavingEditTab; + public bool Visible; private Book.Book _currentlyDisplayedBook; private Book.Book _bookForToolboxContent; @@ -289,7 +295,7 @@ public void UpdateBookDomFromBrowserPageContent(string pageContentData) /// probably the Javascript method that retrieves the page content). /// (Nicer still if cleanup didn't leave the page in an invalid state, see BL-13502.) /// </summary> - private SafeXmlDocument GetCleanCurrentPageFromBodyAndCss( + internal static SafeXmlDocument GetCleanCurrentPageFromBodyAndCss( string bodyHtml, string userCssContent ) @@ -330,7 +336,29 @@ string userCssContent return dom; } - private void SaveCustomizedCssRules(SafeXmlDocument dom, string userCssContent) + /// <summary> + /// Given the combined "body <SPLIT-DATA> userCss" string that the editable-page bundle + /// produces (see captureContentForExternalProcessing / requestPageContent in bloomEditing.ts), + /// build the edited-page HtmlDom ready to hand to Book.SavePage / Book.UpdateDomFromEditedPage. + /// This is the same parsing the live editor does in UpdateBookDomFromBrowserPageContent(string), + /// factored out so the off-screen book processor (external/process-book) can reuse it without + /// going through the live EditingModel/state machine. + /// </summary> + public static HtmlDom GetEditedPageDomFromBrowserContent(string pageContentData) + { + if (pageContentData == null) + throw new ApplicationException("page content was null"); + var endHtml = pageContentData.IndexOf("<SPLIT-DATA>", StringComparison.Ordinal); + if (endHtml < 0) + throw new ApplicationException( + "page content was missing the <SPLIT-DATA> delimiter" + ); + var bodyHtml = pageContentData.Substring(0, endHtml); + var userCssContent = pageContentData.Substring(endHtml + "<SPLIT-DATA>".Length); + return new HtmlDom(GetCleanCurrentPageFromBodyAndCss(bodyHtml, userCssContent)); + } + + private static void SaveCustomizedCssRules(SafeXmlDocument dom, string userCssContent) { // Yes, this wipes out everything else in the head. At this point, the only things // we need in _pageEditDom are the user defined style sheet and the bloom-page element in the body. @@ -356,13 +384,29 @@ private void OnTabAboutToChange(TabChangedDetails details) { if (details.FromTab == Workspace.WorkspaceTab.edit) { + // When an external tool has overwritten the current book on disk (see + // ReloadCurrentBookDiscardingEdits), we are leaving the Edit tab specifically to + // discard the unsaved page. In that case reload from disk instead of saving, so the + // editor's normal save-on-leave doesn't clobber what the external tool just wrote. + var reloadFromDiskInsteadOfSaving = _reloadFromDiskOnLeavingEditTab; + _reloadFromDiskOnLeavingEditTab = false; + SaveThen( () => { // We are setting skipSaveToDisk true so that we can do it ourselves here BEFORE // the postponed work, which is going to shut everything down and would prevent // the normal automatic save-to-disk from working. - CurrentBook?.Save(); // we need it all the way saved before doing the PostponedWork + if (reloadFromDiskInsteadOfSaving) + { + // Discard the page content just gathered into the in-memory DOM; disk wins. + CurrentBook?.ReloadFromDisk(null); + // Force OnBecomeVisible to re-display from the freshly-loaded book if the + // user returns to the Edit tab. + _currentlyDisplayedBook = null; + } + else + CurrentBook?.Save(); // we need it all the way saved before doing the PostponedWork // This bizarre behavior prevents BL-2313 and related problems. // For some reason I cannot discover, switching tabs when focus is in the Browser window // causes Bloom to get deactivated, which prevents various controls from working. @@ -389,6 +433,17 @@ private void OnTabAboutToChange(TabChangedDetails details) { StateMachine.ToNoPage(); } + if (reloadFromDiskInsteadOfSaving) + { + // We reached the fallback because we couldn't take over the save (e.g. a + // save was already in flight: we're in SavePending, waiting on the browser). + // Tell that in-flight save to discard its content, so when it completes it + // doesn't merge the edits we're throwing away and write them back over what + // the external process just put on disk. + StateMachine.DiscardInFlightSave(); + CurrentBook?.ReloadFromDisk(null); + _currentlyDisplayedBook = null; + } _oldActiveForm = Form.ActiveForm; Application.Idle += ReactivateFormOnIdle; details.PostponedWork?.Invoke(); @@ -921,6 +976,49 @@ public void OnBecomeVisible() } } + /// <summary> + /// Reload the currently-selected book from disk, deliberately throwing away any unsaved edits + /// to the page the user might be working on. This is used when an external process (e.g. + /// BloomBridge) has just re-imported/overwritten the book on disk and we want the + /// running Bloom to show the new version. The caller is responsible for making sure this really + /// is the book that was changed; we only ever discard edits for the current selection. + /// If the Edit tab is live, rather than risk reloading the book under the editor mid-edit, we + /// kick the user back to the Collection tab (discarding edits and reloading from disk on the + /// way out); the fresh book is shown if/when they return to the Edit tab. + /// </summary> + public void ReloadCurrentBookDiscardingEdits() + { + var book = CurrentBook; + if (book == null) + return; + + // Make sure we do NOT save the page the user might be editing; we are intentionally + // discarding those edits in favor of what is now on disk. This is the same flag the + // normal book-switch path clears to avoid saving the outgoing page (see OnBookSelectionChanged). + _havePageToSave = false; + + if (!Visible) + { + // The Edit tab isn't showing, so the book isn't live in the browser and there's no + // editing state to unwind. Just reload from disk; OnBecomeVisible will display the + // fresh version when the user next switches to the Edit tab. + book.ReloadFromDisk(null); + _currentlyDisplayedBook = null; + return; + } + + // The Edit tab is showing and the page is live in the browser, very possibly mid-edit. + // Trying to reload-and-renavigate the book in place while the editor is live proved + // fragile (the state machine forbids a direct re-navigation, and unwinding it under the + // user mid-edit could leave the editor in a bad state). The safe, predictable thing is to + // kick the user back to the Collection tab. We set _reloadFromDiskOnLeavingEditTab so the + // leaving-Edit-tab logic in OnTabAboutToChange reloads from disk instead of saving (which + // would clobber the external tool's content). When the user returns to the Edit tab, + // OnBecomeVisible will display the freshly-loaded book. + _reloadFromDiskOnLeavingEditTab = true; + _view.WorkspaceView.ChangeTab(Workspace.WorkspaceTab.collection); + } + /// <summary> /// The code invoked by the state machine to actually start the editable page browser navigating /// to a particular page. Anything that needs saving on the current page should already have been saved. diff --git a/src/BloomExe/Edit/EditingStateMachine.cs b/src/BloomExe/Edit/EditingStateMachine.cs index 86c2624bfb1e..9555dff27e07 100644 --- a/src/BloomExe/Edit/EditingStateMachine.cs +++ b/src/BloomExe/Edit/EditingStateMachine.cs @@ -40,6 +40,11 @@ public class EditingStateMachine private Action<string, string> _updateBookWithPageContents; // args are (pageId, pageContentData) private Action _saveBook; private bool _saveActionHandlesSaveBook; + + // When set, the in-flight save (we are in SavePending, waiting for the browser to return the + // page content) will be discarded on completion rather than merged into the DOM and written to + // disk. See DiscardInFlightSave. + private bool _discardInFlightSave; private Action _hidePage; private Action<bool> _enableStateTransitions; // arg is (enabled) @@ -223,9 +228,14 @@ private void DoPostSaveAction( Action doAfterSaveToDisk = null ) { + // If an external process overwrote the book on disk while this save was in flight, we are + // intentionally discarding the gathered page content (see DiscardInFlightSave): don't merge + // it into the DOM and don't write it to disk below, or we'd clobber what that process wrote. + var discard = _discardInFlightSave; + _discardInFlightSave = false; try { - if (pageContentData != null) + if (pageContentData != null && !discard) { if (pageContentData.StartsWith("ERROR:")) throw new ApplicationException(pageContentData); // This is caught immediately below. We want that error handling for this case. @@ -234,7 +244,7 @@ private void DoPostSaveAction( } else { - // We're in the no page state, and there's nothing to save. + // We're in the no page state (or discarding), and there's nothing to save. } } catch (Exception e) @@ -278,7 +288,7 @@ private void DoPostSaveAction( { _saveActionHandlesSaveBook = false; } - else + else if (!discard) { _saveBook(); doAfterSaveToDisk?.Invoke(); @@ -345,6 +355,23 @@ public bool ToSavePending( } } + /// <summary> + /// If a save is in flight (we are in SavePending, having asked the browser for the current page's + /// content but not yet received it), arrange for that save's completion to throw the content away + /// rather than merging it into the book DOM or writing it to disk. Used when an external process + /// has overwritten the book on disk and we are intentionally discarding the user's unsaved edits + /// (see EditingModel.ReloadCurrentBookDiscardingEdits). Without this, the in-flight save would + /// finish after we reload and clobber the external process's content on disk. + /// Returns true if a save was actually in flight (so the discard will take effect). + /// </summary> + public bool DiscardInFlightSave() + { + if (_currentState != State.SavePending) + return false; + _discardInFlightSave = true; + return true; + } + /// <summary> /// Source: API call providing content of current page will request this after saving and before executing pending action /// (e.g. changing pages) diff --git a/src/BloomExe/IBrowser.cs b/src/BloomExe/IBrowser.cs index 63733a1722e8..fd25e58fdf3c 100644 --- a/src/BloomExe/IBrowser.cs +++ b/src/BloomExe/IBrowser.cs @@ -10,6 +10,7 @@ using Bloom.Book; using Bloom.ToPalaso; using SIL.IO; +using SIL.Windows.Forms.Miscellaneous; namespace Bloom { @@ -308,6 +309,21 @@ public void OnOpenPageInEdge(object sender, EventArgs e) // intentionally letting any errors just escape, give us an error } + /// <summary> + /// Puts the HTML of the current page (the first .bloom-page element) on the clipboard. + /// This is a developer aid for inspecting/reporting page markup. + /// </summary> + public async void OnCopyPageHtml(object sender, EventArgs e) + { + Debug.Assert(!InvokeRequired); + var html = await GetStringFromJavascriptAsync( + "document.getElementsByClassName('bloom-page')[0]?.outerHTML" + ); + // After the await we're back on the UI thread, so it's safe to use the clipboard. + if (!string.IsNullOrEmpty(html)) + PortableClipboard.SetText(html); + } + [Obsolete( "This method is dangerous because it has to loop Application.DoEvents(). RunJavaScriptAsync() is preferred." )] @@ -363,6 +379,11 @@ private void AddOtherMenuItemsForDebugging(IMenuItemAdder adder) "Open Page in Edge", // dev only, no need to localize OnOpenPageInEdge ); + + adder.Add( + "Copy HTML", // dev only, no need to localize + OnCopyPageHtml + ); } public abstract Task SaveDocumentAsync(string path); diff --git a/src/BloomExe/ImageProcessing/ImageUtils.cs b/src/BloomExe/ImageProcessing/ImageUtils.cs index 7645414442e2..182d47d65d42 100644 --- a/src/BloomExe/ImageProcessing/ImageUtils.cs +++ b/src/BloomExe/ImageProcessing/ImageUtils.cs @@ -355,7 +355,10 @@ private static (Color color, int count)[] GetDominantColors(Bitmap bitmapImage) // steps. For a 40×40 image this gives step=2; for a 20×20 image, step=1. int step = Math.Min( kSampleStep, - Math.Max(1, Math.Min(bitmapImage.Width / kMinGridSize, bitmapImage.Height / kMinGridSize)) + Math.Max( + 1, + Math.Min(bitmapImage.Width / kMinGridSize, bitmapImage.Height / kMinGridSize) + ) ); // Divide the RGB cube into 32-unit bins (5 bits discarded per channel, @@ -377,13 +380,18 @@ private static (Color color, int count)[] GetDominantColors(Bitmap bitmapImage) var bitmapData = bitmapImage.LockBits( new Rectangle(0, 0, imageWidth, imageHeight), ImageLockMode.ReadOnly, - PixelFormat.Format32bppArgb); + PixelFormat.Format32bppArgb + ); try { int stride = bitmapData.Stride; byte[] pixels = new byte[stride * imageHeight]; System.Runtime.InteropServices.Marshal.Copy( - bitmapData.Scan0, pixels, 0, pixels.Length); + bitmapData.Scan0, + pixels, + 0, + pixels.Length + ); for (int j = 0, y = step / 2; y < imageHeight; y += step, ++j) { j = Math.Min(j, 9); @@ -401,7 +409,9 @@ private static (Color color, int count)[] GetDominantColors(Bitmap bitmapImage) // toward the surrounding white/black background and loses most of its // chroma; genuine large-region colors are unaffected. const int kHalfWindow = 2; // (2*2+1)² = 25 pixels - long rSum = 0, gSum = 0, bSum = 0; + long rSum = 0, + gSum = 0, + bSum = 0; for (int dy = -kHalfWindow; dy <= kHalfWindow; dy++) { int py = Math.Min(Math.Max(y1 + dy, 0), imageHeight - 1); @@ -420,9 +430,10 @@ private static (Color color, int count)[] GetDominantColors(Bitmap bitmapImage) int g = (int)(gSum / kWindowArea); int b = (int)(bSum / kWindowArea); - int key = ((r >> kBucketShift) << (2 * kBitsPerBucket)) - | ((g >> kBucketShift) << kBitsPerBucket) - | (b >> kBucketShift); + int key = + ((r >> kBucketShift) << (2 * kBitsPerBucket)) + | ((g >> kBucketShift) << kBitsPerBucket) + | (b >> kBucketShift); if (buckets.TryGetValue(key, out var entry)) buckets[key] = (entry.r + r, entry.g + g, entry.b + b, entry.count + 1); else @@ -444,17 +455,22 @@ private static (Color color, int count)[] GetDominantColors(Bitmap bitmapImage) // large images while the floor of 1 ensures no filtering occurs for small images // (where a single sample can represent a genuine but small color region). int totalSamples = 0; - foreach (var b in buckets.Values) totalSamples += b.count; + foreach (var b in buckets.Values) + totalSamples += b.count; int minCount = Math.Max(1, totalSamples / 1700); return buckets .Where(kv => kv.Value.count >= minCount) - .Select(kv => ( - color: Color.FromArgb( - (int)(kv.Value.r / kv.Value.count), - (int)(kv.Value.g / kv.Value.count), - (int)(kv.Value.b / kv.Value.count)), - kv.Value.count)) + .Select(kv => + ( + color: Color.FromArgb( + (int)(kv.Value.r / kv.Value.count), + (int)(kv.Value.g / kv.Value.count), + (int)(kv.Value.b / kv.Value.count) + ), + kv.Value.count + ) + ) .ToArray(); } diff --git a/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs b/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs index e27fdf1d8229..1d5e794ebeb9 100644 --- a/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs +++ b/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs @@ -159,12 +159,7 @@ bool transparencyOnly // Step 1: quick cache check under lock lock (this) { - if ( - _originalPathToProcessedVersionPath.TryGetValue( - cacheFileName, - out var cached - ) - ) + if (_originalPathToProcessedVersionPath.TryGetValue(cacheFileName, out var cached)) { if ( RobustFile.Exists(cached) diff --git a/src/BloomExe/ProjectContext.cs b/src/BloomExe/ProjectContext.cs index d34344cbfcec..2e62c8bd9c07 100644 --- a/src/BloomExe/ProjectContext.cs +++ b/src/BloomExe/ProjectContext.cs @@ -381,7 +381,12 @@ IContainer parentContainer } var server = parentContainer.Resolve<BloomServer>(); - server.SetCollectionSettingsDuringInitialization(_scope.Resolve<CollectionSettings>()); + var collectionSettings = _scope.Resolve<CollectionSettings>(); + server.SetCollectionSettingsDuringInitialization(collectionSettings); + // Let the application-level CommonApi (which can't take a project dependency in its + // constructor) report this collection's folder via common/instanceInfo, so external tools + // can pick the right running Bloom when several are open. + web.controllers.CommonApi.CurrentCollectionSettings = collectionSettings; server.EnsureListening(); // A few APIs are now registered in the constructor of ApplicationContainer @@ -867,6 +872,11 @@ public void Dispose() // disposed. On the whole, I think it's best to clear the handlers first; that way, // at least we avoid mysterious errors in the API handlers due to disposed objects. server.ApiHandler.ClearProjectLevelHandlers(); + // Clear the project-scoped collection settings we set on the application-level CommonApi + // (see constructor), so common/instanceInfo doesn't report stale collection info after the + // project is closed. A reload disposes this context before creating the next one, which + // sets it again, so clearing here is safe. + web.controllers.CommonApi.CurrentCollectionSettings = null; _scope.Dispose(); } finally diff --git a/src/BloomExe/ToPalaso/ProcessExtra.cs b/src/BloomExe/ToPalaso/ProcessExtra.cs index 537e43f8b58e..32ba3d4f80d0 100644 --- a/src/BloomExe/ToPalaso/ProcessExtra.cs +++ b/src/BloomExe/ToPalaso/ProcessExtra.cs @@ -24,6 +24,29 @@ public class ProcessExtra [DllImport("user32.dll")] public static extern bool SetForegroundWindow(IntPtr hWnd); + [DllImport("user32.dll")] + public static extern IntPtr GetForegroundWindow(); + + [DllImport("user32.dll")] + private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out uint lpdwProcessId); + + [DllImport("kernel32.dll")] + private static extern uint GetCurrentProcessId(); + + /// <summary> + /// True if the given window belongs to this (Bloom) process. Used to decide whether it is safe + /// to steal the OS foreground back: if Bloom currently holds it (e.g. an off-screen browser + /// grabbed it) we may restore the prior window, but if some other application now holds the + /// foreground the user has moved on and we must not yank focus away from them. + /// </summary> + public static bool IsWindowInCurrentProcess(IntPtr hWnd) + { + if (hWnd == IntPtr.Zero) + return false; + GetWindowThreadProcessId(hWnd, out uint processId); + return processId == GetCurrentProcessId(); + } + /// <summary> /// Safely start the process when the program code merely supplies the URL (or a command). /// </summary> diff --git a/src/BloomExe/WebView2Browser.cs b/src/BloomExe/WebView2Browser.cs index 31093fdef934..308b7906836d 100644 --- a/src/BloomExe/WebView2Browser.cs +++ b/src/BloomExe/WebView2Browser.cs @@ -31,6 +31,10 @@ public partial class WebView2Browser : Browser public static string AlternativeWebView2Path; private bool _readyToNavigate; + + // Exposes whether the (async) CoreWebView2 environment initialization has finished. Lets callers + // measure how long that init takes separately from the subsequent navigation (see BookProcessor). + public override bool IsReadyToNavigate => _readyToNavigate; private PasteCommand _pasteCommand; private CopyCommand _copyCommand; private UndoCommand _undoCommand; @@ -259,6 +263,50 @@ CoreWebView2ContextMenuRequestedEventArgs e static int dataFolderCounter = 0; + // When set (via BeginSharedEnvironmentBatch), all WebView2 browsers created during the batch + // share ONE CoreWebView2Environment — i.e. one browser process, one user-data folder, and one + // HTTP cache — instead of each creating its own. The first browser of the batch creates the + // environment; the rest reuse it. Each browser still gets its own fresh CoreWebView2 control + // (fresh renderer), so this does NOT reintroduce the single-control reuse-wedge. Used by + // BookProcessor's off-screen per-page fix-up to avoid paying environment creation per page. + // + // These statics assume the batch is UI-thread-only (which it is: all browser construction is + // marshalled to the UI thread, and BookProcessor drives the batch there). If another browser + // happens to be created during the batch (e.g. a thumbnail), it harmlessly joins the shared + // environment. They are NOT a mechanism for concurrent batches. + private static bool _useSharedEnvironment; + private static CoreWebView2Environment _sharedEnvironment; + + public static void BeginSharedEnvironmentBatch() + { + AssertSharedEnvironmentStaticsAreUiThreadOnly(); + _useSharedEnvironment = true; + _sharedEnvironment = null; + } + + public static void EndSharedEnvironmentBatch() + { + AssertSharedEnvironmentStaticsAreUiThreadOnly(); + _useSharedEnvironment = false; + // Drop our reference; the underlying browser process/profile is released once the last + // CoreWebView2 using this environment is disposed. (CoreWebView2Environment is not IDisposable.) + _sharedEnvironment = null; + } + + // The shared-environment statics above have no synchronization; they are safe only because the + // batch is UI-thread-only (see the comment on _useSharedEnvironment). Assert that assumption so + // any future code that drives a batch off the UI thread trips here instead of silently corrupting + // state. Unit-test/console modes are exempt, as elsewhere in this file. + private static void AssertSharedEnvironmentStaticsAreUiThreadOnly() + { + Debug.Assert( + Program.RunningOnUiThread + || Program.RunningUnitTests + || Program.RunningInConsoleMode, + "Shared WebView2 environment batch must be driven on the UI thread (these statics are unsynchronized)" + ); + } + private async Task InitWebView() { // based on https://stackoverflow.com/questions/63404822/how-to-disable-cors-in-wpf-webview2 @@ -378,25 +426,36 @@ private async Task InitWebView() // different instances of WebView2 using the same one, so we decided to make sure it is uniqiue. // Enhance: it might be a good thing to try to delete this folder if we find it already exists (on a background thread). // For now we'll just keep incrementing until we find an available folder. - string dataFolder; - do - { - dataFolder = Path.Combine( - Path.GetTempPath(), - "Bloom WV2-" - + (BloomServer.portForHttp == 0 ? 8085 : BloomServer.portForHttp) - + dataFolderCounter++ - ); - } while (Directory.Exists(dataFolder)); // This sets up a handler for the CoreWebView2InitializationCompleted event, which will run before // EnsureCoreWebView2Async returns if we're awaiting properly, so we need to set up that handler - // before calling EnsureCoreWebView2Async. + // before calling EnsureCoreWebView2Async. (It must run even when we reuse a shared environment, + // because EnsureCoreWebView2Async still fires this control's own InitializationCompleted, which + // is what sets _readyToNavigate.) SetupEventHandling(); - var env = await CoreWebView2Environment.CreateAsync( - browserExecutableFolder: AlternativeWebView2Path, - userDataFolder: dataFolder, - options: op - ); + // During a shared-environment batch, reuse the one environment (and its browser process + + // user-data folder + HTTP cache) so we don't pay environment creation per browser. The first + // browser of the batch falls through and creates it; subsequent ones reuse it. + var env = _useSharedEnvironment ? _sharedEnvironment : null; + if (env == null) + { + string dataFolder; + do + { + dataFolder = Path.Combine( + Path.GetTempPath(), + "Bloom WV2-" + + (BloomServer.portForHttp == 0 ? 8085 : BloomServer.portForHttp) + + dataFolderCounter++ + ); + } while (Directory.Exists(dataFolder)); + env = await CoreWebView2Environment.CreateAsync( + browserExecutableFolder: AlternativeWebView2Path, + userDataFolder: dataFolder, + options: op + ); + if (_useSharedEnvironment) + _sharedEnvironment = env; + } await _webview.EnsureCoreWebView2Async(env); // Added as a footnote to BL-15466 to prevent popups generated from title // attributes being white on black, presumably because of some setting the diff --git a/src/BloomExe/web/controllers/CommonApi.cs b/src/BloomExe/web/controllers/CommonApi.cs index 809cdeab9916..1f283f5761aa 100644 --- a/src/BloomExe/web/controllers/CommonApi.cs +++ b/src/BloomExe/web/controllers/CommonApi.cs @@ -8,6 +8,7 @@ using System.Windows.Forms; using Bloom.Api; using Bloom.Book; +using Bloom.Collection; using Bloom.Edit; using Bloom.MiscUI; using Bloom.web; @@ -34,6 +35,12 @@ public class CommonApi // Autofac was not able to pass us one. public static WorkspaceView WorkspaceView { get; set; } + // The collection settings of the currently-open project, or null if no project is open. + // CommonApi is an application-level handler (created before any project is open and reused + // across projects), so it must NOT take CollectionSettings as a constructor dependency. + // Instead the project scope sets this when it opens a collection (see ProjectContext). + public static CollectionSettings CurrentCollectionSettings { get; set; } + // Called by autofac, which creates the one instance and registers it with the server. public CommonApi(BookSelection bookSelection) { @@ -258,6 +265,11 @@ private void HandleInstanceInfo(ApiRequest request) executableDirectory = Path.GetDirectoryName(executablePath), httpPort = BloomServer.portForHttp, webSocketPort = BloomServer.WebSocketPort, + // The folder of the editable collection this instance has open. An external tool that + // has written/updated a book folder can find the right Bloom (when several are running) + // by matching the parent of its book folder against this path. + editableCollectionFolder = CurrentCollectionSettings?.FolderPath, + collectionName = CurrentCollectionSettings?.CollectionName, serverUrl = BloomServer.ServerUrl, serverUrlWithBloomPrefix = BloomServer.ServerUrlWithBloomPrefixEndingInSlash, workspaceTabsUrl = BloomServer.ServerUrlWithBloomPrefixEndingInSlash @@ -617,11 +629,19 @@ public void HandleBubbleLanguages(ApiRequest request) { var bubbleLangs = new List<string>(); bubbleLangs.Add(LocalizationManager.UILanguageId); - bubbleLangs.Add(_bookSelection.CurrentSelection.BookData.MetadataLanguage1Tag); - if (_bookSelection.CurrentSelection.Language2Tag != null) - bubbleLangs.Add(_bookSelection.CurrentSelection.Language2Tag); - if (_bookSelection.CurrentSelection.Language3Tag != null) - bubbleLangs.Add(_bookSelection.CurrentSelection.Language3Tag); + // CurrentSelection can be null when no book is selected, e.g. when an off-screen + // editable page (external/process-book) loads and requests bubble languages while + // the book it is processing is not the selected book. Fall back to just the major + // languages in that case rather than crashing. + var currentBook = _bookSelection.CurrentSelection; + if (currentBook?.BookData != null) + { + bubbleLangs.Add(currentBook.BookData.MetadataLanguage1Tag); + if (currentBook.Language2Tag != null) + bubbleLangs.Add(currentBook.Language2Tag); + if (currentBook.Language3Tag != null) + bubbleLangs.Add(currentBook.Language3Tag); + } bubbleLangs.AddRange(new[] { "en", "fr", "sp", "ko", "zh-Hans" }); // If we don't have a hint in the UI language or any major language, it's still // possible the page was made just for this langauge and has a hint in that language. @@ -629,7 +649,8 @@ public void HandleBubbleLanguages(ApiRequest request) // Definitely wants to be after UILangage, otherwise we get the surprising result // that in a French collection these hints stay French even when all the rest of the // UI changes to English. - bubbleLangs.Add(_bookSelection.CurrentSelection.BookData.Language1.Tag); + if (currentBook?.BookData?.Language1 != null) + bubbleLangs.Add(currentBook.BookData.Language1.Tag); // if it isn't available in any of those we'll arbitrarily take the first one. request.ReplyWithJson(JsonConvert.SerializeObject(new { langs = bubbleLangs })); } diff --git a/src/BloomExe/web/controllers/ExternalApi.cs b/src/BloomExe/web/controllers/ExternalApi.cs index cbd201523b42..473193851308 100644 --- a/src/BloomExe/web/controllers/ExternalApi.cs +++ b/src/BloomExe/web/controllers/ExternalApi.cs @@ -1,6 +1,13 @@ -using System; +using System; using Bloom.Api; +using Bloom.Book; +using Bloom.Collection; +using Bloom.CollectionTab; +using Bloom.Edit; +using Bloom.web; using Bloom.WebLibraryIntegration; +using Bloom.Workspace; +using SIL.Reporting; namespace Bloom.web.controllers { @@ -12,11 +19,89 @@ public class ExternalApi public static event EventHandler LoginSuccessful; private BloomLibraryBookApiClient _bloomLibraryBookApiClient; + private readonly CollectionModel _collectionModel; + private readonly EditingModel _editingModel; + private readonly WorkspaceTabSelection _tabSelection; + private readonly BookServer _bookServer; + private readonly CollectionSettings _collectionSettings; + + // Re-entrancy guard for process-book. ProcessBook occupies the UI thread but pumps the + // Windows message loop via Application.DoEvents() (both the pre-loop below and the per-page + // waits in BookProcessor). Because external/* handlers are dispatched on the UI thread via + // message posts, a second process-book request that arrives mid-run could be delivered + // re-entrantly during one of those DoEvents pumps. The shared-environment statics in + // WebView2Browser (_useSharedEnvironment/_sharedEnvironment) are explicitly NOT designed for + // overlapping batches, so we reject the re-entrant call rather than let the two corrupt each + // other. A plain bool is sufficient: everything here is single-threaded on the UI thread, so + // there is no cross-thread race to lock against. + private bool _processBookInProgress; + + // The most recent (or in-progress) external/process-book job. process-book replies + // immediately with a jobId and runs the heavy work asynchronously on the UI thread; the + // client then polls external/process-book-status until State is "done" or "failed". This + // replaces a single long-held HTTP response — which a stale/dropped keep-alive socket could + // silently swallow, leaving the client hung in "Converting…" forever — with a pollable + // terminal state the client can read with short, retryable requests, so it learns precisely + // when processing finished. Guarded by _processBookJobLock: the UI thread writes it while + // server worker threads read it for the status endpoint. + private readonly object _processBookJobLock = new object(); + private ProcessBookJob _processBookJob; + + private class ProcessBookJob + { + public string JobId; + public string State; // "running" | "done" | "failed" + public int Processed; + public string BookFolderPath; + public string HtmPath; + public string Error; + } + + private struct ProcessBookResult + { + public int Processed; + public string BookFolderPath; + public string HtmPath; + } // Called by autofac, which creates the one instance and registers it with the server. - public ExternalApi(BloomLibraryBookApiClient bloomLibraryBookApiClient) + public ExternalApi( + BloomLibraryBookApiClient bloomLibraryBookApiClient, + CollectionModel collectionModel, + EditingModel editingModel, + WorkspaceTabSelection tabSelection, + BookServer bookServer, + CollectionSettings collectionSettings + ) { _bloomLibraryBookApiClient = bloomLibraryBookApiClient; + _collectionModel = collectionModel; + _editingModel = editingModel; + _tabSelection = tabSelection; + _bookServer = bookServer; + _collectionSettings = collectionSettings; + } + + /// <summary> + /// BloomBridge is not supported on Team Collections. Operating on a TC safely would require + /// honoring checkout state, preserving each book's TeamCollection.status file, and telling the + /// TC about renames (and recording them in history) — none of which the external write + /// endpoints do. Rather than silently corrupt a shared collection, the mutating endpoints + /// (add/update/process-book) fail fast when the open collection is a Team Collection. Returns + /// true (and fails the request) if we refused; false if it is safe to proceed. + /// </summary> + private bool RefuseIfTeamCollection(ApiRequest request, string endpoint) + { + if (_collectionModel.IsEditableCollectionATeamCollection) + { + request.Failed( + endpoint + + " is not supported on Team Collections. BloomBridge can only be used with a " + + "regular (non-Team) collection." + ); + return true; + } + return false; } public void RegisterWithApiHandler(BloomApiHandler apiHandler) @@ -54,7 +139,7 @@ public void RegisterWithApiHandler(BloomApiHandler apiHandler) false ); - // This is called from bloomlibrary.org after a successful logout. + // This is called from outside Bloom (e.g. bloomlibrary.org) to bring the Bloom window to the front. apiHandler.RegisterEndpointHandler( "external/bringToFront", request => @@ -73,6 +158,745 @@ public void RegisterWithApiHandler(BloomApiHandler apiHandler) }, false ); + + // Called by an external utility (e.g. BloomBridge) after it has written or + // overwritten a book folder in this collection on disk. We make the running Bloom show the + // current state of that book: a brand-new book is added to the collection list; a re-imported + // existing book has its display refreshed. If the re-imported book happens to be the one open + // in the Edit tab, we throw away any unsaved edits and reload it from disk. + // + // This must run on the UI thread because it can reload the Edit tab's view. + apiHandler.RegisterEndpointHandler( + "external/update-book", + HandleUpdateBook, + handleOnUiThread: true + ); + + // Called by an external utility to make the running Bloom select a particular book in the + // collection (the one whose 'id' is supplied). This changes the current selection just as if + // the user had clicked the book in the collection list. + // + // We only honor this when the Collection tab is active. Changing the selection while the user + // is mid-edit would silently discard their unsaved page edits (EditingModel.OnBookSelectionChanged + // clears _havePageToSave and tears down the live editor) and could leave the Edit tab in a bad + // state. So if we're not on the Collection tab, we ignore the request rather than risk havoc. + // + // This must run on the UI thread because changing the selection updates the UI. + apiHandler.RegisterEndpointHandler( + "external/select-book", + HandleSelectBook, + handleOnUiThread: true + ); + + // Called by an external utility (e.g. BloomBridge's "keep this book" flow) to + // copy a book folder from an arbitrary location on disk into the open collection and select + // it. The source folder need NOT be in the collection; it is copied in (the source is left + // untouched), the collection list is reloaded so the new book appears, and it becomes the + // current selection. The reply includes the new book's 'id' so the caller can later target + // it with external/select-book or external/update-book. + // + // Like external/select-book this reloads the collection and changes the selection, so we only + // honor it while the Collection tab is active (otherwise we'd risk discarding the user's + // unsaved edits). This must run on the UI thread because it updates the UI. + apiHandler.RegisterEndpointHandler( + "external/add-book", + HandleAddBook, + handleOnUiThread: true + ); + + // Called by an external utility (e.g. BloomBridge) to run the full "make it + // right" pass on a book in this collection (the one whose 'id' is supplied): bring it + // structurally up to date, then process every page off-screen in a real browser the same + // way visiting each page in the Edit tab does, and save it to disk. This is the step that + // applies the browser-only fix-ups (image sizing, canvas-element layout, etc.) that raw + // generated HTML is missing. The call blocks until processing is complete. + // + // This must run on the UI thread because it creates and pumps an off-screen WebView2. + // + // requiresSync: false is essential here. The off-screen editable page this handler loads + // makes its own /bloom/api/* calls during bootstrap (image/info to size images, bubble + // languages, etc.). If we held the global API sync lock for the whole ~20-30s run, those + // dependent requests (which also default to requiresSync) would block behind us while we + // block waiting for them to complete the page — a deadlock. We don't need the global lock + // anyway: process-book is already serialized against itself by _processBookInProgress, and + // the user is blocked behind the opaque ExternalBusyOverlay for the duration, so there is no + // competing user-driven API traffic to race with. + apiHandler.RegisterEndpointHandler( + "external/process-book", + HandleProcessBook, + handleOnUiThread: true, + requiresSync: false + ); + + // Poll the status of the most recent external/process-book job (see HandleProcessBook). + // Read-only, so it does not need the UI thread — and crucially it MUST answer on a server + // worker thread while the UI thread is busy processing a book, so the client can watch for + // completion mid-run. requiresSync: false for the same reason process-book uses it: while a + // book processes, the off-screen WebView2's own /bloom/api/* bootstrap calls hold the global + // sync lock, and a status poll must not queue behind them — it only reads an in-memory field + // under its own lock. Returns { state: "unknown" | "running" | "done" | "failed", ... }. + apiHandler.RegisterEndpointHandler( + "external/process-book-status", + HandleProcessBookStatus, + handleOnUiThread: false, + requiresSync: false + ); + + // Called by an external utility (e.g. BloomBridge) to discover which languages + // the open collection is set up for, so it can tag the book content it generates correctly. + // Returns the collection's L1/L2/L3 language tags; L3Code is null when the collection has no + // third language. This is read-only, so it does not need the UI thread. + apiHandler.RegisterEndpointHandler( + "external/collection-languages", + HandleCollectionLanguages, + handleOnUiThread: false + ); + } + + private void HandleCollectionLanguages(ApiRequest request) + { + if (request.HttpMethod == HttpMethods.Options) + { + // Allow a CORS preflight request to succeed (as the other external endpoints do). + request.PostSucceeded(); + return; + } + + // Language1/Language2 are always present; Language3 is optional. + request.ReplyWithJson( + new + { + L1Code = _collectionSettings.Language1?.Tag, + L2Code = _collectionSettings.Language2?.Tag, + L3Code = string.IsNullOrEmpty(_collectionSettings.Language3?.Tag) + ? null + : _collectionSettings.Language3.Tag, + } + ); + } + + private void HandleProcessBook(ApiRequest request) + { + if (request.HttpMethod == HttpMethods.Options) + { + // Allow a CORS preflight request to succeed (as the other external endpoints do). + request.PostSucceeded(); + return; + } + if (request.HttpMethod != HttpMethods.Post) + { + request.Failed("external/process-book only supports POST"); + return; + } + + // Reject a process-book that arrives while one is already running (see field comment). + // BloomBridge sends these sequentially so in practice this never trips, but it makes the + // re-entrancy contract explicit and protects the shared-environment statics if it ever does. + if (_processBookInProgress) + { + request.Failed( + "external/process-book is already processing a book; try again later" + ); + return; + } + + string id = null; + string folderPath = null; + bool fitImageTextSplits = false; + try + { + var data = Newtonsoft.Json.Linq.JObject.Parse(request.RequiredPostJson()); + id = (string)data["id"]; + folderPath = (string)data["path"]; + // Optional: auto-fit simple single-image/single-text origami pages (currently both + // image-above-text and image-left-of-text; grow the image pane to fill the space the + // text doesn't need). Defaults to false. + fitImageTextSplits = (bool?)data["fitImageTextSplits"] ?? false; + if (string.IsNullOrEmpty(id) && string.IsNullOrEmpty(folderPath)) + { + request.Failed( + "external/process-book requires a book 'path' (preferred) or 'id'" + ); + return; + } + + // Processing rewrites the book on disk. Only do it from the Collection tab, so we + // never write a book out from under the live editor or fight its save/navigate state + // machine. + if (_tabSelection.ActiveTab != WorkspaceTab.collection) + { + request.Failed( + "external/process-book is only allowed while the Collection tab is active" + ); + return; + } + + if (RefuseIfTeamCollection(request, "external/process-book")) + return; + } + catch (Exception e) + { + // A failure validating/parsing the request (before any work started). Report it the + // old synchronous way — there is no job to poll yet. + Logger.WriteError( + "external/process-book failed to start for book " + (folderPath ?? id), + e + ); + request.Failed("external/process-book failed: " + e.Message); + return; + } + + // The heavy work needs the UI thread (it creates and pumps an off-screen WebView2). We + // are on the UI thread now, but we must NOT do that work here: holding the HTTP response + // open for the whole ~20-30s run is exactly what leaves the client hung if the connection + // is dropped (e.g. a stale keep-alive socket). Instead reply immediately with a jobId and + // BeginInvoke the work to run after this response is sent — it queues on this same UI + // thread's message loop and runs once this handler returns. The client polls + // external/process-book-status for the outcome. _processBookInProgress stays true for the + // whole async run, so the re-entrancy guard still rejects an overlapping process-book. + var shell = Shell.GetShellOrNull(); + if (shell == null || shell.IsDisposed) + { + request.Failed("external/process-book: Bloom has no main window to process on."); + return; + } + + var jobId = Guid.NewGuid().ToString("N"); + _processBookInProgress = true; + lock (_processBookJobLock) + { + _processBookJob = new ProcessBookJob { JobId = jobId, State = "running" }; + } + request.ReplyWithJson(new { jobId, state = "running" }); + + shell.BeginInvoke( + (Action)(() => RunProcessBookJob(jobId, folderPath, id, fitImageTextSplits)) + ); + } + + /// <summary> + /// Runs the heavy process-book work on the UI thread. Posted via BeginInvoke from + /// HandleProcessBook so it executes after that request's reply has already been sent, then + /// records the outcome on _processBookJob for the client to poll via + /// external/process-book-status. Never throws to the caller (it is a fire-and-forget UI + /// message); any failure is captured as the job's "failed" state. + /// </summary> + private void RunProcessBookJob( + string jobId, + string folderPath, + string id, + bool fitImageTextSplits + ) + { + try + { + // The overlay 'show', the DoEvents/Sleep spin-up loop, and the processing all run + // inside this try so that an exception anywhere after we raise the overlay still runs + // the finally and sends 'hide'; otherwise the modal overlay would be stuck opaque + // until the user navigates away. (Sending 'hide' when 'show' never succeeded is a + // harmless no-op.) + try + { + // Let the user know Bloom is busy. The work below pumps the message loop via + // Application.DoEvents(), so the main WebView2 keeps painting and this overlay + // (with its CSS spinner) stays visible/animated for the whole run. Pump a few + // events first so it actually appears before the heavy work ties up the thread. + dynamic overlay = new DynamicJson(); + // Intentionally NOT localized, like the add-book/update-book toasts: this is an + // operator-facing message shown only during a BloomBridge-driven processing run. + overlay.message = "Bloom is processing a book for BloomBridge, please wait…"; + BloomWebSocketServer.Instance?.SendBundle( + "externalProcessing", + "show", + overlay + ); + for (var i = 0; i < 10; i++) + { + System.Windows.Forms.Application.DoEvents(); + System.Threading.Thread.Sleep(15); + } + + var result = !string.IsNullOrEmpty(folderPath) + ? ProcessBookByPath(folderPath, fitImageTextSplits) + : ProcessBookById(id, fitImageTextSplits); + + lock (_processBookJobLock) + { + if (_processBookJob?.JobId == jobId) + { + _processBookJob.State = "done"; + _processBookJob.Processed = result.Processed; + _processBookJob.BookFolderPath = result.BookFolderPath; + _processBookJob.HtmPath = result.HtmPath; + } + } + } + finally + { + BloomWebSocketServer.Instance?.SendEvent("externalProcessing", "hide"); + } + } + catch (Exception e) + { + Logger.WriteError("external/process-book failed for book " + (folderPath ?? id), e); + lock (_processBookJobLock) + { + if (_processBookJob?.JobId == jobId) + { + _processBookJob.State = "failed"; + _processBookJob.Error = e.Message; + } + } + } + finally + { + _processBookInProgress = false; + } + } + + /// <summary> + /// Report the state of the most recent process-book job. The client polls this (with short, + /// independent requests) until State is a terminal "done"/"failed", rather than waiting on one + /// long-held process-book response. An optional jobId query param scopes the answer to a + /// specific job: if it doesn't match the current job we report "unknown" so a stale poll can't + /// misread a newer job's result. + /// </summary> + private void HandleProcessBookStatus(ApiRequest request) + { + if (request.HttpMethod == HttpMethods.Options) + { + // Allow a CORS preflight request to succeed (as the other external endpoints do). + request.PostSucceeded(); + return; + } + + var jobId = request.GetParamOrNull("jobId"); + lock (_processBookJobLock) + { + if ( + _processBookJob == null + || (!string.IsNullOrEmpty(jobId) && _processBookJob.JobId != jobId) + ) + { + request.ReplyWithJson(new { state = "unknown" }); + return; + } + request.ReplyWithJson( + new + { + jobId = _processBookJob.JobId, + state = _processBookJob.State, + processed = _processBookJob.Processed, + bookFolderPath = _processBookJob.BookFolderPath, + htmPath = _processBookJob.HtmPath, + error = _processBookJob.Error, + } + ); + } + } + + /// <summary> + /// Process a book given the path to its folder, which need NOT be a member of the open + /// collection. The book is processed in place (the fixed-up .htm is written back to the same + /// folder) using the running project's CollectionSettings (xmatter/branding/languages), and + /// nothing is added to the open collection. This is what BloomBridge uses, so its staging + /// books no longer have to be copied into the collection just to be processed. + /// </summary> + private ProcessBookResult ProcessBookByPath(string folderPath, bool fitImageTextSplits) + { + if ( + !System.IO.Directory.Exists(folderPath) + || !SIL.IO.RobustFile.Exists(System.IO.Path.Combine(folderPath, "meta.json")) + ) + { + throw new ApplicationException( + "external/process-book could not find a book folder (with meta.json) at " + + folderPath + ); + } + + // Normally the path points at an off-screen staging folder that is NOT the selected book, + // so there's no live editor state to reconcile. But a caller could hand us the folder of the + // book currently open in the Edit tab; capture that now (before processing, which may rename + // the folder) so we can reload the editor afterward and not let it clobber what we wrote. + // isInEditableCollection:true + AlwaysEditSaveContext makes Book.IsSaveable true + // (Book.IsSaveable => IsInEditableCollection && BookInfo.IsSaveable), matching the semantics + // of the old flow where the book was first copied into the editable collection. + var selectedBeforeProcessing = _collectionModel.GetSelectedBookOrNull(); + var processingSelectedBook = + selectedBeforeProcessing != null + && AreSameFolder(selectedBeforeProcessing.FolderPath, folderPath); + + var bookInfo = new BookInfo(folderPath, true, new AlwaysEditSaveContext()); + var book = _bookServer.GetBookFromBookInfo(bookInfo); + var pageCount = BookProcessor.ProcessBook(book, fitImageTextSplits); + + // If the folder we just rewrote is the book currently open in the Edit tab, the live + // EditingModel still holds the pre-processed in-memory DOM; the next time the user leaves the + // Edit tab, OnTabAboutToChange would Save() it and silently overwrite what we just wrote. + // Discard those in-memory edits and reload from disk, then refresh the collection's view of + // it. This mirrors ProcessBookById. It only reconciles in-memory UI state, so a failure + // here is logged but does NOT turn the job into a failure (the disk output is already correct). + if (processingSelectedBook) + { + try + { + _editingModel.ReloadCurrentBookDiscardingEdits(); + _collectionModel.ReloadEditableCollection(); + } + catch (Exception e) + { + Logger.WriteError( + "external/process-book: book at " + + folderPath + + " was processed and saved, but the in-memory UI refresh afterward failed", + e + ); + } + } + + // Saving may have renamed the folder to match the book's title, so report the final + // location the client should read its output from. + return new ProcessBookResult + { + Processed = pageCount, + BookFolderPath = book.FolderPath, + HtmPath = book.GetPathHtmlFile(), + }; + } + + /// <summary> + /// True if the two paths refer to the same folder on disk (normalized, trailing separators and + /// case ignored — Windows file systems are case-insensitive). Used to tell whether a path-based + /// process-book is targeting the book currently open in the Edit tab. + /// </summary> + private static bool AreSameFolder(string a, string b) + { + if (string.IsNullOrEmpty(a) || string.IsNullOrEmpty(b)) + return false; + return string.Equals( + System + .IO.Path.GetFullPath(a) + .TrimEnd( + System.IO.Path.DirectorySeparatorChar, + System.IO.Path.AltDirectorySeparatorChar + ), + System + .IO.Path.GetFullPath(b) + .TrimEnd( + System.IO.Path.DirectorySeparatorChar, + System.IO.Path.AltDirectorySeparatorChar + ), + StringComparison.OrdinalIgnoreCase + ); + } + + /// <summary> + /// Legacy flow: process a book that is a member of the open editable collection, found by its + /// bookInstanceId. + /// </summary> + private ProcessBookResult ProcessBookById(string id, bool fitImageTextSplits) + { + var editableCollection = _collectionModel.TheOneEditableCollection; + var collectionPath = editableCollection.PathToDirectory; + var bookInfo = _collectionModel.BookInfoFromCollectionAndId(collectionPath, id); + if (bookInfo == null) + { + // The book may have just been written to disk and our in-memory collection cache + // doesn't know about it yet. Rescan from disk and look again before giving up. + _collectionModel.ReloadEditableCollection(); + bookInfo = _collectionModel.BookInfoFromCollectionAndId(collectionPath, id); + } + if (bookInfo == null) + { + throw new ApplicationException( + "external/process-book could not find a book with id " + id + ); + } + + // Process a fresh Book read from disk rather than any in-memory selection, so we + // don't disturb the state of the currently-selected book object. + var book = _bookServer.GetBookFromBookInfo(bookInfo); + var pageCount = BookProcessor.ProcessBook(book, fitImageTextSplits); + + // The book is now processed and saved on disk: the operation the caller asked for has + // succeeded. Everything below only reconciles in-memory UI state, so wrap it so a refresh + // failure is logged but does NOT fail the job — otherwise we'd report failure for a book + // whose output is already correct on disk, and the caller would discard it. + try + { + // If we just rewrote the book that is currently selected, the in-memory selection now + // disagrees with disk (we processed a separate Book instance). Discard the stale in-memory + // copy and reload it from disk so a later trip through the Edit tab can't clobber what we + // just wrote, then refresh the collection's view of it (list metadata + thumbnail). This + // mirrors how external/update-book handles re-import of the selected book. + var selected = _collectionModel.GetSelectedBookOrNull(); + if (selected != null && selected.ID == id) + { + _editingModel.ReloadCurrentBookDiscardingEdits(); + _collectionModel.ReloadEditableCollection(); + var refreshedInfo = _collectionModel.BookInfoFromCollectionAndId( + collectionPath, + id + ); + if (refreshedInfo != null) + _collectionModel.UpdateThumbnailAsync( + _collectionModel.GetBookFromBookInfo(refreshedInfo) + ); + } + } + catch (Exception e) + { + Logger.WriteError( + "external/process-book: book " + + id + + " was processed and saved, but the in-memory UI refresh afterward failed", + e + ); + } + + return new ProcessBookResult + { + Processed = pageCount, + BookFolderPath = book.FolderPath, + HtmPath = book.GetPathHtmlFile(), + }; + } + + private void HandleAddBook(ApiRequest request) + { + if (request.HttpMethod == HttpMethods.Options) + { + // Allow a CORS preflight request to succeed (as the other external endpoints do). + request.PostSucceeded(); + return; + } + if (request.HttpMethod != HttpMethods.Post) + { + request.Failed("external/add-book only supports POST"); + return; + } + + string folderPath = null; + try + { + // Parse with Newtonsoft rather than Bloom's DynamicJson because the body contains a + // Windows path, and DynamicJson's JSON->XML conversion chokes on the backslashes. + var data = Newtonsoft.Json.Linq.JObject.Parse(request.RequiredPostJson()); + folderPath = (string)data["path"]; + if (string.IsNullOrEmpty(folderPath)) + { + request.Failed("external/add-book requires a book 'path'"); + return; + } + + // Adding a book reloads the collection and changes the current selection, which would + // discard the user's unsaved edits if they were mid-edit. Only do it from the Collection + // tab, matching external/select-book and external/process-book. + if (_tabSelection.ActiveTab != WorkspaceTab.collection) + { + request.Failed( + "external/add-book is only allowed while the Collection tab is active" + ); + return; + } + + if (RefuseIfTeamCollection(request, "external/add-book")) + return; + + var newBook = _collectionModel.AddBookFromFolder(folderPath); + if (newBook == null) + { + request.Failed( + "external/add-book copied the book but could not locate it in the collection afterward" + ); + return; + } + + // Intentionally NOT localized: operator-facing notification driven by an external tool. + var timestamp = DateTime.Now.ToString("h:mm:ss tt"); + ToastService.ShowToast( + text: $"Added book \"{newBook.Title}\" ({timestamp})", + durationSeconds: 180 + ); + + // Report the new book's id and final on-disk location so the caller can target it later. + request.ReplyWithJson( + new + { + id = newBook.ID, + bookFolderPath = newBook.FolderPath, + htmPath = newBook.GetPathHtmlFile(), + } + ); + } + catch (Exception e) + { + Logger.WriteError("external/add-book failed for path " + folderPath, e); + request.Failed("external/add-book failed: " + e.Message); + } + } + + private void HandleSelectBook(ApiRequest request) + { + if (request.HttpMethod == HttpMethods.Options) + { + // Allow a CORS preflight request to succeed (as the other external endpoints do). + request.PostSucceeded(); + return; + } + if (request.HttpMethod != HttpMethods.Post) + { + request.Failed("external/select-book only supports POST"); + return; + } + + string id = null; + try + { + // Parse with Newtonsoft rather than Bloom's DynamicJson for consistency with updateBook + // (DynamicJson's JSON->XML conversion can choke on Windows paths and other content). + var data = Newtonsoft.Json.Linq.JObject.Parse(request.RequiredPostJson()); + id = (string)data["id"]; + if (string.IsNullOrEmpty(id)) + { + request.Failed("external/select-book requires a book 'id'"); + return; + } + + // Only change the selection when the Collection tab is active. If the user is editing + // or publishing, quietly ignore the request rather than discard their work or disrupt + // their current tab. We still report success so the caller isn't treated as an error. + if (_tabSelection.ActiveTab != WorkspaceTab.collection) + { + Logger.WriteEvent( + "external/select-book ignored for book id " + + id + + " because the Collection tab is not active (ActiveTab=" + + _tabSelection.ActiveTab + + ")" + ); + request.PostSucceeded(); + return; + } + + var editableCollection = _collectionModel.TheOneEditableCollection; + var collectionPath = editableCollection.PathToDirectory; + var bookInfo = _collectionModel.BookInfoFromCollectionAndId(collectionPath, id); + if (bookInfo == null) + { + request.Failed("external/select-book could not find a book with id " + id); + return; + } + + _collectionModel.SelectBook(_collectionModel.GetBookFromBookInfo(bookInfo)); + + request.PostSucceeded(); + } + catch (Exception e) + { + Logger.WriteError("external/select-book failed for book id " + id, e); + request.Failed("external/select-book failed: " + e.Message); + } + } + + private void HandleUpdateBook(ApiRequest request) + { + if (request.HttpMethod == HttpMethods.Options) + { + // Allow a CORS preflight request to succeed (as the login endpoint does). + request.PostSucceeded(); + return; + } + if (request.HttpMethod != HttpMethods.Post) + { + request.Failed("external/update-book only supports POST"); + return; + } + + string id = null; + try + { + // Note: we parse with Newtonsoft rather than Bloom's DynamicJson because the body + // typically contains a Windows folderPath, and DynamicJson's JSON->XML conversion + // throws on the backslashes in such paths. + var data = Newtonsoft.Json.Linq.JObject.Parse(request.RequiredPostJson()); + id = (string)data["id"]; + if (string.IsNullOrEmpty(id)) + { + request.Failed("external/update-book requires a book 'id'"); + return; + } + + if (RefuseIfTeamCollection(request, "external/update-book")) + return; + + var editableCollection = _collectionModel.TheOneEditableCollection; + var collectionPath = editableCollection.PathToDirectory; + var bookInfo = _collectionModel.BookInfoFromCollectionAndId(collectionPath, id); + + bool added = bookInfo == null; + + if (added) + { + // A new book appeared on disk. Rescan the collection so it shows up in the list, + // then locate it so we can name it in the toast and build its thumbnail. + _collectionModel.ReloadEditableCollection(); + bookInfo = _collectionModel.BookInfoFromCollectionAndId(collectionPath, id); + } + else + { + // The book already existed and has been re-imported/overwritten on disk. + var selected = _collectionModel.GetSelectedBookOrNull(); + if (selected != null && selected.ID == id) + { + // It's the book currently open in the Edit tab. Discard any unsaved edits to it + // and reload it from disk. We do NOT touch the editor for any other book, so a + // user editing an unrelated book never loses work. + // Note: when the Edit tab is live this schedules an async tab-switch (the actual + // switch completes via PostponedWork after the browser returns page content). The + // collection reload below is safe to run immediately only because it doesn't read + // tab/selection state. + _editingModel.ReloadCurrentBookDiscardingEdits(); + } + // Re-read the collection so the list (titles, sort order) reflects the new content, + // then refresh the thumbnail below. + _collectionModel.ReloadEditableCollection(); + bookInfo = _collectionModel.BookInfoFromCollectionAndId(collectionPath, id); + } + + if (bookInfo == null) + { + // Even after reloading the collection we cannot find a book with this id on disk. + // The update did not land, so report failure rather than a false-positive + // "Added/Updated" notification that would hide the real problem from the caller. + request.Failed("external/update-book could not find a book with id " + id); + return; + } + + string title = bookInfo.Title ?? bookInfo.QuickTitleUserDisplay ?? ""; + // GetBookFromBookInfo returns the current selection (already reloaded above) when this + // is the selected book, otherwise a fresh Book read from disk; either way the thumbnail + // reflects the new content. + _collectionModel.UpdateThumbnailAsync( + _collectionModel.GetBookFromBookInfo(bookInfo) + ); + + // Intentionally NOT localized: this is a developer/operator-facing notification driven by + // an external automation tool. We include a timestamp and keep the toast up for a few + // minutes so the user can see, after the fact, that (and when) an external update landed. + var timestamp = DateTime.Now.ToString("h:mm:ss tt"); + var verb = added ? "Added" : "Updated"; + var message = $"{verb} book \"{title}\" ({timestamp})"; + ToastService.ShowToast(text: message, durationSeconds: 180); + + request.PostSucceeded(); + } + catch (Exception e) + { + Logger.WriteError("external/update-book failed for book id " + id, e); + request.Failed("external/update-book failed: " + e.Message); + } } } } diff --git a/src/BloomExe/web/controllers/ProblemReportApi.cs b/src/BloomExe/web/controllers/ProblemReportApi.cs index 4b735571594d..585ff839fcd9 100644 --- a/src/BloomExe/web/controllers/ProblemReportApi.cs +++ b/src/BloomExe/web/controllers/ProblemReportApi.cs @@ -1058,10 +1058,6 @@ bool isShortMessagePreEncoded private const uint PW_RENDERFULLCONTENT = 0x00000002; - /// <summary>Returns the handle of the window currently in the foreground.</summary> - [DllImport("user32.dll")] - private static extern IntPtr GetForegroundWindow(); - /// <summary>Returns the thread and process that created the given window.</summary> [DllImport("user32.dll")] private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out uint lpdwProcessId); @@ -1072,7 +1068,7 @@ bool isShortMessagePreEncoded /// </summary> private static bool IsBloomProcessInForeground() { - var fgWindow = GetForegroundWindow(); + var fgWindow = ProcessExtra.GetForegroundWindow(); if (fgWindow == IntPtr.Zero) return false; GetWindowThreadProcessId(fgWindow, out uint fgProcessId); diff --git a/src/content/templates/xMatter/Null-XMatter/Null-XMatter.less b/src/content/templates/xMatter/Null-XMatter/Null-XMatter.less new file mode 100644 index 000000000000..88053cf8e930 --- /dev/null +++ b/src/content/templates/xMatter/Null-XMatter/Null-XMatter.less @@ -0,0 +1,5 @@ +// The Null xmatter pack intentionally produces no front/back matter pages, so it +// needs no styling. This (essentially empty) file exists only because Bloom requires +// every xmatter pack to have a matching <packname>-XMatter.css stylesheet +// (see XMatterHelper); without it, loading a book that uses this pack throws. +@XMatterPackName: "Null";