Repository navigation
Honor browser preferences and modifiers for HTML preview links - #33622
Conversation
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: applying browser preferences and keyboard modifiers to HTML preview links. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The description is comprehensive and covers the user-visible change, rationale, linked issue STA-11774, testing results, visual QA evidence, compatibility constraints, security boundaries, and known l… |
Full details: Description check
-
Explanation
The description is comprehensive and covers the user-visible change, rationale, linked issue STA-11774, testing results, visual QA evidence, compatibility constraints, security boundaries, and known limitations. It does not use the template headings or checkboxes, and it does not explicitly address AI disclosure, but these omissions do not prevent review because the required substantive information is present.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
fc414761-c3ca-454a-8e82-e96d53d8393d
-
src/main/browser/doc-preview-guest-policy.test.tssrc/renderer/src/components/browser-pane/workspace-doc/doc-preview-external-link-confirmation.test.tssrc/renderer/src/lib/http-link-routing-store.test.tssrc/renderer/src/lib/http-link-routing.tssrc/renderer/src/lib/workspace-browser-tab-open.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
-
|
HTML preview links used a separate opener after confirmation, so they ignored the browser preference and keyboard modifiers and could target a newly selected workspace. Confirmed links now use the common router with the preview's captured workspace and grant owner. Ordinary clicks follow the browser destination preference; Cmd+Shift on macOS / Ctrl+Shift on Windows/Linux uses the existing alternate-destination setting. The exact-URL confirmation remains. Disposing the source preview cancels a pending action. Older servers and explicit SSH device browsing retain their desktop provider; remote failures never retry elsewhere. Three review rounds are complete. The final independent decider accepted the owner-required preview entry, existing-caller adapter and one shared executor. Review fixes add one common shell-open failure surface, real-store moved-path/provider/failure contracts, listener scaling and cleanup coverage, and the required-owner boundary. No open P0/P1 remains. A possible legacy getter timing difference has no demonstrated production failing input and is deferred; no extra guard was added. CI is green on QA: Real paired, historical document-capable server and managed-SSH previews reached rendered marker pages. Confirmation/cancel, alternate modifiers, SVG/shadow anchors, sibling browsing and blocked scripted egress have actual flow evidence. Windows covered the ordinary system destination and both Ctrl+Shift inversion settings: the real opener received each exact confirmed URL, with no new Orca destination. The isolated Windows account has no associated browser, so its original opener rejected the request. System browser rendering not observed; handoff to the OS verified. The final modifier case directly showed one “Unable to open URL.” message. The earlier two cases’ delayed captures do not establish their transient-message behavior; they are counted as routing evidence. A genuine baseline build with the same false preference and plain click rendered its marker inside Orca, reproducing the original preference bug. Normal workspace selection disposed the branch preview and cancelled its pending action. The live-preview/competing-workspace ownership boundary is covered by contracts, rather than claimed as a positive mounted-preview UI race run. Raw SSH-owned document reads are not served on either build; real SSH QA used the managed-server route. Fragment clicks left the hash unchanged on both builds. Restored client-hosted pages became unavailable after a test-client restart; fresh ordinary routing has separate rendered proof. These observations are not claimed fixed by this PR. For Brennan's review:
| Main managed SSH comparison | Human judgment spots: capture source and retain confirmation, required owner and one routing executor, live-grant admission and optional click metadata. Retained Mac routing QA head: |
381 commits landed on main between fd25eb0 and dc73505. All seven catalogs and en-runtime-required.json conflicted, so they are taken from main and the prune is recomputed in one pass: the previous 1,637 keys plus 4, none left the set. stablyai#30449 dropped the readers of RepositorySourceControlAiActionRows.7a3a8e431d and SourceControlAiActionRecipeDefaults.2cb4bb7e5d and added settings.sourceControl.actionModel.permissionsDefaultHint without one; stablyai#33622 dropped the reader of auto.hooks.ipc.events.browserStateIpcBridge.docPreviewLinkFailed. Two keys exist only in en.json and pt.json never had permissionsDefaultHint, so en.json sheds 1,641, pt.json 1,638 and the other five catalogs 1,639. en.json goes from 17,395 entries to 15,754; en-runtime-required.json keeps 691 of them. main deleted both test files this branch edited: catalog-nested-settings.test.ts (stablyai#34136) and native-chat-shared-copy-matches-catalog.test.ts (stablyai#34411). The deletions are taken, so the branch now differs from main in 13 files.
…nk preference (#39190) Since #33622 a confirmed HTML preview link honors openLinksInApp (default off, so it opens the system browser) and no longer calls openBrowserProfileTabInActiveWorkspace, so the spec's store spy never fired. Opt the client into in-app links and observe the opened page URL directly. Route doc-preview sources to the spec.
HTML document preview links always opened Orca Browser after confirmation, ignoring the browser preference and keyboard modifiers. They could also open under a workspace selected after the original click.
Confirmed links now use the shared HTTP router with the preview's captured workspace and grant owner. Ordinary clicks honor the configured destination. Cmd+Shift on macOS / Ctrl+Shift on Windows/Linux uses the existing alternate-destination and inversion settings; Cmd/Ctrl alone remains ordinary routing. The router also applies existing profiles and matching-page reuse. Disposing the source preview cancels a pending action.
Exact-URL confirmation and the existing trusted-click, live-grant and HTTP(S) boundaries remain. Optional modifier metadata is additive desktop IPC; paired-server file-reading protocols are unchanged. Older servers and explicit SSH device browsing retain their existing desktop provider, without retrying elsewhere after a remote failure. Intended differences from the common pattern are grant-scoped reads, HTTP(S)-only egress, exact confirmation and existing multi-host compatibility. No temporary implementation or follow-up dependency.
Tracked in STA-11774.
Validation:
325a49fced675aa7fd2acf4076777e14443c1a92, including typechecks, test shards, packages, managed-SSH E2E and all WSL distro checks. The cancelled Ubuntu WSL job passed its targeted retry.Existing limitations observed on both builds: raw SSH-owned document reads are not served, so real SSH preview QA used the normal managed-server route; fragment clicks left the guest hash unchanged. Main reproduced wrong-workspace routing. The branch's normal workspace switch disposes the preview and cancels its pending action; captured-source race behavior is covered by contracts rather than claimed as a positive mounted-preview UI race. Restored client-hosted pages became unavailable after test-client restart; fresh ordinary routing and the original modifier's rendered proof are recorded separately. No fix to those unrelated existing surfaces is claimed.