Skip to content

Keep the main window on the app's own page, and draw a mail in a frame of its own - #571

Merged
zaerl merged 5 commits into
trunkfrom
fix/main-window-navigation
Oct 8, 2026
Merged

zaerl merged 5 commits into
trunkfrom
fix/main-window-navigation

Conversation

@zaerl

@zaerl zaerl commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Why

Two things about the app's main window were left open, and this closes both as a security hardening.

The main window was not held to the page the app loads into it: it could be navigated or redirected to another page, and it could open a second window. The Trac window has been pinned to its origin since it was written (pinToTrac in src/trac-view.js); the main window had nothing of the kind.

And a mail's HTML part was put into the app's own document, so markup the app did not write became part of the page: its styles applied to the whole window, and its links and forms acted on the window itself.

What changes

The main window stays on its own page. pinToOwnPage in the new src/window-navigation.js cancels every navigation away from the page, cancels a main-frame redirect, and denies every new window. An address the window refuses is handed to src/external-url.js, the gate url:open already uses: http and https open in the system browser, anything else is refused and logged. A page reloading itself is still allowed. createWindow() calls it, and the hand-off to the browser is one function in src/main.js shared with url:open.

A mail's HTML part is drawn in a frame of its own, an <iframe srcdoc> with an empty sandbox: no scripts, no forms, no new windows, no navigating the window around it. The frame inherits the window's content security policy, which needed no change for the frame to render and keeps scripts and remote loads blocked inside it. A link in a mail does nothing, and the mail stays on screen. The frame asks for the light colour scheme, so a mail is drawn on a white page with dark text in either theme, the way a mail client draws one, and fills its box to the edge.

The rule is written down where it is checked. react/no-danger is on for the renderer, so a new dangerouslySetInnerHTML fails lint, and the review standard has a paragraph on both halves.

Deliberately not in this PR: the content security policy is unchanged, links in a mail are not sent to the browser, and no other window is touched.

How to test this

Platforms: any. Use the Buildkite build for the current head of this branch.

Three journeys carry this, run from the repository root:

  • npx playwright test --project=journeys -g "a middle click on a link to the site"
  • npx playwright test --project=journeys -g "a mail is read inside the app"
  • npx playwright test --project=journeys -g "a mail the site sent"

All three are red on trunk's code. The first is red without the pin (a second app window opens and the address never reaches the browser). The second is red when the mail is drawn in the dialog's own document, when the frame is given more than the empty sandbox, and when a link replaces the mail in its frame. The third now reads the rendered mail through its frame.

What they cannot reach is a real dev server and a mail a real site sent, since they stub the first and seed the second. The middle click needs a mouse with a clickable wheel; a trackpad has no middle button.

Starting state: a site that has finished setup.

  1. Click Start development server and wait for View site, wp-admin and Database to appear in the site's details. The app opens the site in your browser by itself at this point; close that tab.
  2. Middle-click each of the three links. Each opens in your browser, and Adminer keeps its full query string.
  3. Plain-click each of the three. Same as before: each opens in your browser.
  4. In wp-admin, send a mail: on the login screen, Lost your password? for the admin user.
  5. Back in the app, click Toggle Email at the bottom of the window and click the mail in the tray. It reads as it did before, Rendered and Raw both.
  6. If you have a plugin that sends HTML mail, open one of its mails. It reads as a mail, in the app's font unless it sets its own. Click a link in it: nothing happens, and the mail is still there.
  7. In the settings, switch to the dark theme and open the same HTML mail. It is on a white page with dark text that fills the bordered box, with no dark band around it.

What must not have happened:

  • No window of the app shows the WordPress site, after any click.
  • The app window still shows the app, never WordPress and never an empty page.
  • The dialog's heading and tabs look the same whatever the mail's own styles say.

Risks and limitations

  • A link in a mail does nothing. That is deliberate and recorded as a characterisation in the journey. Sending it to the browser is a separate decision.
  • Escape does not close the mail dialog after a click inside an HTML mail, because the keyboard focus is then in the frame. The Close button works, and so does Escape before clicking in the mail.
  • An HTML mail scrolls inside its frame, which is 60vh tall, instead of stretching the dialog. It no longer inherits the dialog's styles; it starts from the app's font and size on a white page, in either theme, and its own markup wins from there.
  • Overlap with Open links in the browser for every click gesture, not just a plain click #358, which addresses Links can open inside the app instead of the browser #284 by taking http and https navigation and new windows to the browser. This pin does that and also refuses every other scheme. The two touch the same lines in createWindow() and url:open, so they need to be reconciled rather than both merged as they are.
  • I ran the journeys and both unit passes on macOS only. Windows is CI's. The dark-theme drawing was checked in a standalone Electron page reproducing the frame, not in the app: the journeys run in the light theme only.
  • Rebased onto trunk at 9e48b4d (the redesign, translations and themes landed meanwhile). The rebased head had a fresh-context review; see the review outcome.
  • Review: 3 [fix here], all fixed, and 3 [follow-up], listed in the collapsed block. A second review of the rebased head found 2 more [fix here], both fixed, and no new follow-up.

Related

Related to #284 and #358. Follows #569, which moved the mail dialog into its own component.


Design decisions and alternatives considered

The page the window may stay on is the one it is on, read from the window when the event arrives, not an address fixed in main.js. Both strings come from Chromium, so there is no path to normalise across platforms and no second copy of where the app's page lives.

The pin takes one function, openInBrowser, and decides nothing about schemes. Which addresses may reach the OS stays in external-url.js, in one place, and a refused one is logged there the same way as for url:open.

will-redirect acts on the main frame only. Electron sends it for frames inside the page too. A frame redirecting is not the window leaving, and its address is not one to hand to the browser.

An empty sandbox, and no sanitiser. A sanitiser is a list of what is dangerous that has to stay complete; the frame is a boundary that does not depend on what the mail contains. It also needs no new dependency.

A <base target="_blank"> in front of the mail. Without it a link with no target loads in the frame itself, the frame may not load it, and the mail is replaced by an empty page. Aimed at a new window, which the frame may not open either, the link does nothing and the mail stays. A srcdoc document is always in standards mode, so putting it in front of a doctype changes nothing about how the mail is laid out.

The content security policy was checked, not changed. A srcdoc frame renders under default-src 'self' as it stands and inherits the policy.

Review outcome (required — see AGENTS.md)

3 [fix here] · 3 [follow-up]. All 3 fixed.

  • Review: completed. Reviewer: a separate agent context with no access to the authoring session, given the diff and .github/instructions/code-review.instructions.md. Reviewed head 90098aa, base 3270f30 (trunk, no pull request existed yet). npm run lint clean; npm test and npm run test:electron pass; all journeys pass on macOS.
  • Since review: 90098aa → bf568e8 is the three fixes below and nothing else. Lint, both unit passes and all journeys re-run on bf568e8.
  • Rebase, 2026-10-08: the three commits were replayed onto trunk at 9e48b4d (now fb7674c, 74eeff6, 6f2d7d8). Conflicts were in AGENTS.md, TESTING.md, tests/e2e/helpers/ui.cjs and mail.spec.js, resolved to trunk's wording plus this PR's additions; the frame journey now opens the Email tray first. d286d96 follows: the frame asks for the light scheme and carries its padding, its size moves to shell.css for the redesign's lint, and its title is translated. On d286d96: lint clean, npm test and npm run test:electron pass, the mail and site-link journeys pass on macOS, and the site-link journey is red with the pin disabled.
  • Review of the rebase: completed. Reviewer: a separate agent context given only the diff and .github/instructions/code-review.instructions.md. Reviewed head d286d96, base 9e48b4d (trunk). npm run lint clean, npm test passes. Outcome: 2 [fix here] · 1 [follow-up], all 🔵; both fixed in 4f8edb0:
    • Tests, low: in mail.spec.js, the check that the second mail shows nothing of the first searched the dialog only, which does not enter the frame the first one's HTML part is drawn in. It now also checks the second mail has no frame. Red with a stale frame drawn, where the old line stayed green.
    • Tests, low: the frame's translated title was read by no journey, since unwrapped() collects no title. i18n.spec.js now checks it, and is red with the title left untranslated.
    • Follow-up, low: a hyperlink in the terminal. Already listed below; the reviewer confirmed that on trunk it opened a second window with the preload bridge, and that with the pin it does nothing after its confirmation.
    • Also in 4f8edb0: the comment over MAIL_DEFAULTS (a mail's styles win, its <base> does not), AGENTS.md and TESTING.md naming every journey that stubs the server's start, and a sentence in the review standard's §5 on content moved into a frame.
    • On 4f8edb0: lint clean; the mail, site-link and i18n mail journeys pass on macOS. 4f8edb0 changes only tests, a comment and documentation, so it was checked against the findings and not reviewed again.

Fixed:

  • Tests, low: the wp-admin link's locator was written in two end-to-end specs. It is named once in tests/e2e/helpers/ui.cjs now.
  • Tests, low: a wiring test asserted that the pin ran before loadFile, an order nothing can observe. Removed; the test with the real modules already fails when the main window is not pinned.
  • Architecture, low: will-redirect also fires for frames, and the pin would have treated one as the window leaving. It acts on the main frame only now, with a test.

Follow-ups, each a decision rather than an omission:

  • Medium: links in a mail do nothing. Kept on purpose for this PR; see Risks.
  • Low: a hyperlink printed in the in-app terminal uses the terminal library's default handler, which asks for a new window. With the pin that window is denied, so the link does nothing after its confirmation. To be routed through url:open separately.
  • Low: one finding in a window this PR does not touch, which predates the branch and is being handled on its own.

CodeRabbit: not requested yet.

Implementation notes
  • src/window-navigation.js: pinToOwnPage(webContents, { openInBrowser }).
  • src/main.js: openInBrowser(url) wraps openExternalUrl; createWindow() pins the window and logs a browser that could not be opened; url:open calls the same function.
  • src/renderer/components/email-modal.jsx: the frame and what goes in front of every mail. src/renderer/shell.css: the frame's size, and a framed mail's box without padding.
  • eslint.config.mjs: react/no-danger for src/renderer/.
  • tests/unit/window-navigation.test.cjs (layer 1), tests/unit/ipc-wiring.test.cjs (layer 3: the stub window now records its navigation listeners and its new-window handler), tests/e2e/journeys/mail.spec.js and tests/e2e/journeys/site-links.spec.js (layer 4).
  • AGENTS.md and TESTING.md add site-links.spec.js (and the two already missing, auto-start.spec.js and i18n.spec.js) to the journeys that press "Start development server" with the server stubbed; the Electron-pin guidance is unchanged.
  • Checked on Electron 43.6.0: a reload from the menu and Playwright's page.reload() do not raise will-navigate, so neither is affected; location.reload() raises it with the page's own address, which is the one navigation the pin lets through.
Screenshots or recording

Nothing new appears on screen. A plain-text mail is drawn exactly as before, and an HTML mail is drawn in the same place in the dialog, on a white page in either theme.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: dc850c1e-31be-4ee4-a3d4-4f008596b9e7

📥 Commits

Reviewing files that changed from the base of the PR and between 2695d4c and a3a1be1.

📒 Files selected for processing (13)
  • .github/instructions/code-review.instructions.md
  • AGENTS.md
  • TESTING.md
  • eslint.config.mjs
  • src/main.js
  • src/renderer/components/email-modal.jsx
  • src/window-navigation.js
  • tests/e2e/helpers/ui.cjs
  • tests/e2e/journeys/mail.spec.js
  • tests/e2e/journeys/site-links.spec.js
  • tests/e2e/real-setup/create-site.spec.js
  • tests/unit/ipc-wiring.test.cjs
  • tests/unit/window-navigation.test.cjs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The main window now stays on its current page and routes other navigation attempts through external URL handling. Email HTML now renders in an empty-sandbox iframe instead of in the app document. The changes add unit and end-to-end coverage for these behaviors and update renderer lint rules and testing guidance.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to a3a1b

The change keeps outside navigation out of the app window and isolates HTML mail. Inactive mail links are an intentional limitation. No concrete issue requiring correction before merge remains; normal checks still apply.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a3a1b

The changes reduce exposure by keeping the app window on its own page and separating email content from the app interface. No introduced security weakness was established. Remaining uncertainty concerns runtime enforcement of the email frame boundary and failure-path validation.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The relevant exposure is the contributor's local desktop session: opening attacker-authored mail causes its HTML to be parsed, while navigation attempts can reach the system browser through the main process. The protected authority is the app window's preload bridge; accepted external targets remain limited by scheme, not destination host.

Security Findings and Attack Paths

  • inferred — The changes narrow the previously described paths from mail markup into app-wide styling and navigation, and from app-window navigation into another page carrying the preload bridge. Source comparison and inspected controls support hardening rather than an introduced bypass; the mail journey contains containment assertions but was not executed in this review.

Trust Boundaries and Controls

  • inferred — The empty sandbox separates mail from parent-document scripting, form submission, popup creation, and top-level navigation authority. Resource containment additionally depends on the inherited app CSP, rather than the navigation helper policing subframes. The guard's trusted-page identity is dynamic, so it relies on main-process loading remaining restricted to the app page.

Resilience and Maintainability Implications

  • inferred — Navigation decisions are synchronous and have no shared mutable authorization state. Repeated attempts do not accumulate permission, and rejected browser-opening promises are logged without reversing app-window denial. Repetition can still initiate multiple external handoffs; concurrent handoff and operating-system failure behavior remain unvalidated by the inspected tests.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description includes all required sections, explains the security changes and deliberate limitations, provides platform-specific test steps and expected outcomes, records review evidence, lists re…
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zaerl zaerl self-assigned this Sep 30, 2026
@zaerl

zaerl commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

zaerl added a commit that referenced this pull request Oct 5, 2026
## Why

`AGENTS.md`, in its note about the Electron pin, says the Playground
server crashes "about a second after "Start dev server"" and that the
packaged smoke "never clicks "Start dev server"". Since the redesign
(#595) the control
in the page's header and in the details reads "Start development
server". Someone following the note to check an Electron bump would look
for a button that is not there.
#617 corrected the
same words in `TESTING.md` and left this file for a decision, since it
is the project's instructions.

## What changes

Two words, twice, in one line of `AGENTS.md`: "Start dev server" becomes
"Start development server". Nothing else.

The setup checklist's own button still reads "Start dev server and
finish the wizard"; that is a different control, and the note is about
starting a server on a site that is set up.

## How to test this

Platforms: any. Nothing runs differently: it is prose. To see that the
name is the control's:

```
git grep -n "Start development server" src/renderer/site-processes.cjs tests/e2e/helpers/ui.cjs
```

## Risks and limitations

- Review: not run by a separate reviewer. See below.
- #571 changes one
line of `AGENTS.md` too; if it is the same line, one of the two will
need the other's words when trunk is merged across.

## Related

Part of #542. Follows
#617, before
#591.

---

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

- **Review:** not run — the change is two words repeated in one line of
prose, at the maintainer's request. I checked that the line is still one
line, that the name is the one `src/renderer/site-processes.cjs` gives
the control, and that no other tracked doc still uses the old name (`git
grep "Start dev server" AGENTS.md TESTING.md CONTRIBUTING.md .github`).
- **Since review:** none.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
zaerl added a commit that referenced this pull request Oct 5, 2026
## Why

#542 replaces the app's interface with the one in the
[prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype),
drawn with the WordPress Design System. It was built in slices on
`redesign/wpds`, so that trunk, which releases are cut from, never held
a half-redesigned window. The window is whole now: the shell, the page
header and details, every card and dialog, and the tray. This pull
request is how the branch reaches trunk.

It lands before the last three sub-issues, on purpose. The settings
dialog (#559) and dark mode (#560) add to a finished window and do not
need a branch to hide in, and the guide (#561) is easier to rewrite
against what is on trunk. They follow as pull requests against trunk.

## What changes

Everything `redesign/wpds` holds; this pull request adds nothing of its
own. Each slice was reviewed in its own pull request against that
branch, and says there how to test it and what it leaves out.

- **The shell** (#555): a sites list, the page area and a footer, in a
larger window. #590
- **The page header and the details** (#556): the site's menu in the
header, and what the checkout is in a sidebar that can be put away.
#594
- **The site's page** (#557): the server and the build watch in the
header and the details
(#595); the ticket
card, the apply card with a preview dialog, and the list of a site's
tickets (#596,
#597,
#599); the
create-site, rename and delete dialogs
(#600,
#601);
confirmations as notices in the window's corner
(#602); the page's
notices, update card and setup checklist
(#605); the
terminal, logs, mail and the review dialogs painted with the design
system's tokens
(#606,
#608); and lint
rules that keep it so
(#609,
#610).
- **The tray** (#558): along the bottom of the window, holding the
terminal, the logs or the mail, with a count of unseen debug.log lines
on the footer
(#612,
#613,
#614,
#615).
- Along the way:
#611 (the site
menu's update item says why it is held), and three that tidy up before
landing: #616 (a
journey's race with the page's clock),
#617 (comments the
finished redesign made untrue) and
#618 (the server's
button by its present name in `AGENTS.md`).

**What a contributor will notice first:** the terminal, the logs and the
mail are no longer on the page but behind three buttons at the foot of
the window; the terminal is dark text on light; the server and the build
watch are started from the header; a patch is previewed in a dialog
before it is applied; and what worked is said in the window's corner.

**Not in this pull request:**

- The settings dialog (#559). Until it, nothing about how the window is
arranged is remembered across restarts: the details, the tray's height,
the apply card's fold.
- Dark mode (#560). The terminal reads its colours once, when it is
made, and will need to follow the theme.
- The user guide and its screenshots (#561), which describe the old
interface. They are published from release tags, so they are not wrong
until a release ships this; #561 is next, ahead of #559 and #560, for
that reason.
- How a mail is drawn, and how the window handles a link. Both are
unchanged and tracked separately.

## How to test this

Platforms: macOS and Windows, both. Buildkite builds signed artifacts
for this pull request; use the build for its current head, and check
that it is: a push to the branch makes earlier builds stale.

The suite, from the repository root, after `npm ci`:

```
npm run lint
npm test
npm run test:e2e
```

113 journeys walk every slice on macOS and Windows on each push. What
they do not do is run a real site: a clone of `wordpress-develop`, an
install, a build and a server are stood in for. The one test that does,
`real-setup.yml`, is run by hand and was run on this branch at `548c020`
(https://github.com/WordPress/contributor-toolkit/actions/runs/37298667056):
a Core site on macOS and on Windows and a Gutenberg site on macOS were
created, installed, built and served, and passed. **The Gutenberg site
on Windows did not finish.** Its first attempt was cancelled after 74
minutes inside the journey, a minute short of the limit that would have
failed it with evidence, so it says nothing about where it was; a second
attempt was still running when this was written and its result is not in
this description. The commits since `548c020` change a journey, comments
and prose.

**A pass by hand, on a real machine** (any platform; Apple Silicon for
the server, which no test on a pull request starts for real). It follows
the order a contributor meets things in:

1. Open the app with no sites. **Create new site**, a Core site, in a
folder with a space in its path. The setup checklist runs its steps; the
terminal's button at the foot of the window shows what each is doing.
2. Let the checklist's last step start the server, and finish
WordPress's own setup in the browser. Back in the app the header says
"Server running" and "Build watching". Stop the server and start it
again: from **Server running** in the header, or with **Stop** and
**Start** beside "Server" in the details.
3. Press the three buttons at the left of the footer in turn. Terminal:
type `help`. Logs: the server's lines, the build watch's, and Debug.log.
Email: where SMTP is listening. Drag the tray's top edge.
4. Link a ticket by its number. The card shows the ticket and its pull
requests and patches.
5. Apply one of them. The preview dialog names the files; **Apply and
rebuild** runs the steps on the card.
6. Edit a file under `src/`, then **Review & submit changes**. The diff
is the edit and nothing else.
7. Create a second site, or open one from before, and move between the
two from the sites list. Each has its own terminal, logs and mail.
8. **Update to latest trunk** from the site's menu.

**What must not have happened:** work lost in a link, an apply, a switch
or an update that did not say so; `node_modules` rebuilt where nothing
changed; a failure said only in a tray that stayed closed; one site's
output in another's tray; anything on the page under the tray.

**What cannot be tested by hand, and what was not:** none of the slices'
manual steps were run by a person. I looked at screenshots and drove the
app through the journeys; everything local was on macOS, with Windows
left to CI. This pass is the first one by hand.

## Risks and limitations

- **It is one commit on trunk**, since the repository allows only squash
merges. The slices' pull requests are its record. `redesign/wpds` should
not be branched from after this: it will have diverged from trunk.
- **Reviewed in slices, not as a whole.** Twenty-two pull requests, all
but a two-word change to `AGENTS.md` reviewed by a separate reviewer
context before they were opened, most in two passes; see below. The
branch was then swept once for what falls between slices, which found
comments and one unread field
(#617) and the
three points that follow.
- **Three questions and two failures still use the system's own
dialogs**: the discards ask through its confirm box, and a path that
could not be copied and a server started before there is a build are
said in its alert. #557 meant to replace them and did so for deletions
only.
- **Some dialogs are repainted, not rebuilt.** The review dialog and its
destinations, the mail dialog and the question an update asks about
loose edits have the design system's colours and type on the old
component library's parts.
- **A real Gutenberg setup on Windows is not shown to pass on this
branch.** See "How to test this". It passed on trunk before the
redesign, in 35 minutes; here it was past 46 on its second attempt.
Whether that is the runner or the branch is not known.
- **The guide describes the old interface** until #561. Do not cut a
release before it.
- **The bundle roughly doubled**
(#590): the sites
list brings `DataViews` and what it needs. Launch time was not measured.
- **Less is always in view.** A build's output, a new line in debug.log
and a caught mail are in a tray that may be closed. A failure opens the
terminal, or the logs if nothing else is open; unseen debug.log lines
are counted on the footer; a mail that arrives shows nowhere until its
tray is opened, which was decided and is not an oversight.
- **Translation is about half done.** The shell, the cards, the tray and
most dialogs go through the translator. Roughly 200 strings in
components and hooks do not yet (the review dialog and its destinations,
the setup checklist, the logs, most of what the hooks say), nor about
150 in the modules they take sentences from. Strings that are new are in
English in the shipped translations until they are translated.
- **Focus is lost in a few places**: when the app swaps what the tray
holds from under it, when a notice with a focused button goes, when a
row's own action removes the row.
- **Some failures are still worded by the main process** ("Error
invoking remote method…"): a failed rename, a failed setup.
- **Things carried over, not made by this:** a start that throws leaves
the server's button dead until a restart; erasing typed input in the
terminal breaks once the line wraps; a line written to debug.log while
it is being attached to can be shown twice.
- **#571 and
#603 will conflict
with trunk after this**, in the mail's journey and helpers and in the
dev server's hook.
- Each slice's own list has more, and says which of its limits were
chosen.

## Related

Closes #557. Closes #558. Part of #542, which stays open for #559, #560
and #561. #555 and #556 are closed already.

---

<details>
<summary>Design decisions and alternatives considered</summary>

- **An integration branch, and one landing.** A window drawn half by the
old view and half by the new one could not be released, and the slices
were too many to hold back one by one behind a flag. Fixes and changes
with no visible effect went to trunk first and were merged across, so
this diff is the redesign and little else.
- **Landing before #559 to #561.** The plan was to land when all seven
sub-issues were in the branch. The remaining three do not change what is
there: two add to it and one documents it. Holding a finished window on
a branch for them only lengthens the time trunk and the branch can drift
apart.
- **What the prototype lacks is kept and redrawn**, and what it has
somewhere else moves to where it puts it. Its labels are adopted. Where
a slice departs from it, the slice says so.

</details>

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

**No review of the whole diff was run. The record is the slices':
twenty-two pull requests into `redesign/wpds`, each but one reviewed
before it was opened by a separate, read-only reviewer context following
`.github/instructions/code-review.instructions.md`, most in two passes
(the change, then its fix-up), with the counts and the findings in each
pull request's own review outcome. The one is
#618, two words in
a line of prose.**

- **Review:** completed, per slice —
#590,
#594 to
#602,
#605,
#606,
#608 to
#617 (no review was
run for #618);
everything marked to fix in a slice was fixed in that slice, and what
was left is in its Risks.
- **Review:** completed, for leftovers only — a separate agent context,
read-only; `redesign/wpds` at `548c020` against trunk; not a review of
correctness, which the slices had: a search for dead CSS and code,
comments that waited for a slice that has landed, what is left of the
old component library and of the system's dialogs, strings not yet
translatable, and repository docs that name the branch. Nothing found in
the stylesheet, the dependencies or the docs; thirteen comments and one
unread field, fixed in
#617; the rest is
in Risks above.
- **CodeRabbit:** not run. Its check reads "Review skipped: automatic
reviews are disabled" on this pull request and on the slices, so the
slices' pre-PR reviews are the reviews of record, each for the head it
names.
- **Since review:** nothing was added to the branch outside a reviewed
slice, apart from three merges of trunk (`da19ac6`, `3344172`,
`23d46e7`).

</details>

<details>
<summary>Implementation notes</summary>

- `git diff origin/trunk...origin/redesign/wpds`: 148 files, about
14,800 lines added and 3,000 removed; about 6,300 of the added lines are
tests.
- The site's view, which was one component of five thousand lines, is
spread over the four regions the layout has: the header, the cards, the
details and the tray. Decisions are in `src/renderer/*.cjs` with unit
tests; components draw them.
- New for the suite: journeys per region under `tests/e2e/journeys/`,
lint rules of the repository's own (`eslint-rules/`), and Stylelint over
the window's stylesheet.
- The main process changed in one place for the redesign's sake: the
debug.log tail says which lines the file already held
(#614).

</details>

<details>
<summary>Screenshots or recording</summary>

Not attached: I have no way to upload images from the command line. The
prototype shows the design; the signed build shows the app.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@zaerl zaerl added this to the v2.0.0 milestone Oct 7, 2026
zaerl and others added 4 commits October 8, 2026 10:57
The main window is the one window with the preload bridge, and nothing
held it to the page the app loads into it: it could be navigated or
redirected to another page, and it could open a second window.

`pinToOwnPage` in src/window-navigation.js cancels every navigation and
redirect away from the page and denies every new window, the way
trac-view.js pins the Trac window. An address the window refuses is
handed to external-url.js, so an http or https link opens in the system
browser and anything else is refused and logged. A page reloading itself
is still allowed.

That is also what #284 asks for: a middle click on a link to the site
opens the browser, not a window of the app.

Tests at three layers: the module with a stand-in webContents, the wire
from `createWindow` in ipc-wiring, and a journey for the middle click.
TESTING.md and AGENTS.md name the second journey that presses "Start dev
server" with the server stubbed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A mail's HTML part was put into the app's own document, so markup the
app did not write became part of the page: its styles applied to the
whole window, and its links and forms acted on the window itself.

The HTML part is now drawn in an `<iframe srcdoc>` with an empty
`sandbox`: no scripts, no forms, no new windows, no navigating the
window around it. The frame inherits the window's content security
policy, which needed no change for it to render and keeps scripts and
remote loads blocked inside it. A link in a mail does nothing, and the
mail stays where it is.

`react/no-danger` is on for the renderer, so a new
`dangerouslySetInnerHTML` fails lint, and the review standard says what
the rule is and why.

mail.spec.js reads the rendered mail through its frame, and a second
journey presses everything in a mail that asks to go somewhere.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- `will-redirect` is also sent for frames inside the page, so the pin
  acts on it for the main frame only: a frame redirecting is not the
  window leaving, and its address is not one to hand to the browser.
- The wiring test that asserted the pin ran before `loadFile` pinned an
  order nothing can observe. The test with the real modules already
  fails when the main window is not pinned, so the first one goes.
- The wp-admin link is reached by two end-to-end specs now, so it is
  named once in tests/e2e/helpers/ui.cjs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…frame's title

The mail's frame said nothing about its colour scheme, so in the dark theme it
was white only because Chromium paints a white page behind a frame whose scheme
differs from the window's, and it sat in the box's dark padding. It now asks
for the light scheme, the way a mail client draws a mail, carries the padding
inside the frame and fills the box to its edge. Its size moves to shell.css,
which the redesign's lint asks for, and its title goes through __().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zaerl
zaerl force-pushed the fix/main-window-navigation branch from 783bd46 to d286d96 Compare October 8, 2026 09:03
The second mail in mail.spec.js was checked for the first one's text on the
dialog only, which does not enter the frame the first one's HTML part is drawn
in; it now checks that it has no frame. The i18n mail journey checks the
frame's title, an attribute unwrapped() does not read. The comment over
MAIL_DEFAULTS said the mail's markup wins, which holds for its styles and not
for its <base>. AGENTS.md and TESTING.md name every journey that stubs the
server's start, and the review standard says what moving content into a frame
does to a locator.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zaerl
zaerl merged commit f5b3b81 into trunk Oct 8, 2026
8 checks passed
@zaerl
zaerl deleted the fix/main-window-navigation branch October 8, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant