Windows: Follow off-site redirects in the tab, not the window

[epeicher]

What it does

Fixes Connect to WooCommerce.com breaking the window it runs in. After this change, clicking Connect account takes the browser tab to WooCommerce.com's sign-in, as classic wp-admin does. When WooCommerce.com sends you back, you land in the desktop with the WooCommerce screen open in a window.

Why

WooCommerce answers the connect request with a redirect to woocommerce.com/…/oauth/authorize. Inside a window, the iframe followed that redirect. woocommerce.com sends X-Frame-Options: SAMEORIGIN, so the window showed the browser's "refused to connect" page and the flow stopped. Any sign-in flow that redirects to another site from an admin page fails the same way.

How

• Server (includes/core/routing.php): a wp_redirect filter catches a window's navigation (chromeless, not a script's request) that redirects to another host. It answers with a small hand-off page instead of the redirect. The page posts os-iframe-offsite-redirect with the destination to the shell, and has a target="_top" "Continue to …" link for a parent that doesn't answer. Same-site redirects (saves, the front end, relative admin URLs) are untouched.
• Shell (src/window/iframe-bridge.ts): on that message, the shell drops the window and does location.assign() to the destination. It drops the window without the unsaved-changes prompt, because its page has already left. A window kept in the session would reopen on the redirecting URL and send the user straight back out.
• Boot guard: if the user hasn't interacted with the shell yet (navigator.userActivation.hasBeenActive === false), the window is only dropped. This happened while testing: a session saved with the broken window restored it at boot and sent the tab to woocommerce.com without a click.
• docs/bridge-protocol.md documents the new message.

Testing instructions

1. Install and activate WooCommerce, then open the desktop.
2. Open WooCommerce → Extensions, then click Connect to WooCommerce.com → Connect account. (Or click Connect under My subscriptions.)
3. The whole tab goes to WooCommerce.com's authorize page. Before this change, a window opened showing a "refused to connect" error.
4. Approve or deny on WooCommerce.com. You return to the desktop with the WooCommerce helper screen in a window, and a reload does not send you back out.
5. Regression check: save a post or a settings screen in a window; it still stays in the window.

Tests: npm run test:php -- --filter=Tests_OpenStationChromelessPreserveRedirect (adds the off-site predicate and hand-off page cases). npm run test:js, lint, typecheck and lint:php are green.

[epeicher]

@claude review this PR

[claude]

Claude finished @epeicher's task in 26s —— View job

---

Review of PR #1008: follow off-site redirects in the tab

I read the diff and checked the surrounding helpers. I did not run the test suites. Overall the approach is sound and well scoped: it covers only navigations, leaves same-site redirects alone, documents the new message, and the shell side drops the window before leaving. I have a few points, mostly minor.

Worth a look

1. The postMessage target origin and the sandbox. The hand-off script posts to window.location.origin and the shell checks event.origin !== INITIAL_ORIGIN. That is fine for a same-origin iframe. The try/catch swallows failures silently, which is reasonable here because the target="_top" link is the fallback.
2. leaveForOffsiteRedirect runs after the os-iframe-offsite-redirect check but is not tied to the sender window. handleWindowMessage( win, event ) is already scoped to a window, so this holds. Please confirm the caller validates event.source against that window's iframe, so a different window's frame cannot make this window destroy itself and navigate the tab. A same-origin frame can post this message anyway, so the real exposure is small: it is a forced navigation to an arbitrary http(s) URL, and any same-origin script could already do that with location.assign.
3. Multisite and sibling hosts. openstation_is_offsite_redirect_target() compares only against site_url() and home_url() hosts. On a network, redirects to another site in the network, such as a sibling subdomain or the network admin, count as off-site. They would now take the tab there rather than failing in the frame. That is probably the right behavior, because docs/multisite.md says those cannot be windows. Hosts added through allowed_redirect_hosts (for example a CDN or SSO host a site trusts) are still treated as off-site. The effect is correct, since they are still cross-origin, but a test or a line in the docblock would make the multisite behavior deliberate.
4. Hook priority and ordering. The new filter runs at 1000, after openstation_chromeless_preserve_redirect (999) and the plugins hand-off (998). So a redirect that the earlier filters rewrite is judged on its rewritten value, which seems intended. The exit inside a wp_redirect filter skips later filters and wp_redirect's own exit. That is fine, but an exit in a filter also bypasses wp_redirect_status, and that is acceptable here.
5. Missing test coverage for the guard conditions. The new tests cover the predicate and the document markup. They do not cover that openstation_chromeless_hand_off_offsite_redirect() returns the location unchanged for a non-chromeless request or a subresource request, which is the most regression-prone part. This is a "could plausibly come back" case, so one short test would be justified. The function calls exit on the hand-off path, so only the pass-through branch is easy to test.
6. A relative or protocol-relative redirect to your own host. The //accounts.example.org/signin case is covered. A scheme-less host such as woocommerce.com/path has no scheme, so wp_parse_url returns a path and no host, and it is correctly treated as non-offsite. That matches how browsers resolve it (as a relative path).

Minor

• openstation_offsite_redirect_document() echoes wp_get_inline_script_tag() output. The JSON uses JSON_HEX_TAG | JSON_HEX_AMP, and the test confirms </script> does not survive. Good. JSON_UNESCAPED_SLASHES is safe with HEX_TAG.
• esc_url() on $location strips some characters. If a destination uses a scheme esc_url rejects, the link renders empty, but the predicate already limits to http(s).
• The status is set to 200 with nocache_headers(). Consider whether 200 is better than a non-redirect error code for logs and monitoring; 200 is fine for an iframe document.
• navigator.userActivation?.hasBeenActive === false skips navigation only when the API is present and reports no activity. Browsers without the API will always navigate, including at boot. That is the safe-for-feature choice, but it means the boot-restore protection is not universal, so it is worth stating in the docblock.
• In docs/bridge-protocol.md, the new row is placed between os-iframe-other-admin-link and os-iframe-admin-link, which is fine. The table cell is very long, and you may want to trim it.
• There are no version-history annotations, so that rule is satisfied. The PHP file-length warning is not a concern here.

Verdict: Looks good to merge once point 2 is confirmed and, ideally, one pass-through test is added for point 5. Nothing here is blocking.

[epeicher]

Thanks for the review. Addressed in 453712b:

1. Target origin and the silent try/catch. No change. The target="_top" link is the fallback, and since 7209159 it stays hidden for a second, so it only shows when the shell did not take the tab.
2. Sender check. Confirmed. handleWindowMessage() returns early unless event.source === win.iframe.contentWindow (and event.origin is the shell's), so another window's frame cannot make this window destroy itself or move the tab.
3. Multisite and allowed_redirect_hosts. Deliberate. The openstation_is_offsite_redirect_target() docblock now says that network siblings and trusted hosts count as off-site, and that the tab follows them because they are other origins that cannot be windows (docs/multisite.md).
4. Priority and exit. No change. Judging the rewritten value at 1000 is intended, and skipping wp_redirect_status is fine for a 200 document.
5. Pass-through coverage. Added test_an_offsite_redirect_outside_a_window_navigation_is_left_alone. It checks that the location comes back unchanged for a non-chromeless request and for a chromeless request with Sec-Fetch-Mode: cors.
6. Scheme-less host. No change. As noted, it stays a relative path.

Minor

• navigator.userActivation: the leaveForOffsiteRedirect() docblock now says that a browser without the API cannot tell a boot restore from a click, and always follows.
• docs/bridge-protocol.md: the os-iframe-offsite-redirect row is trimmed to the contract.
• Status 200 and esc_url(): left as is, for the reasons you gave.