Add setting to open non-localhost links in Integrated Browser#316530
Add setting to open non-localhost links in Integrated Browser#316530Copilot wants to merge 8 commits into
Conversation
Co-authored-by: jruales <1588988+jruales@users.noreply.github.com>
Co-authored-by: jruales <1588988+jruales@users.noreply.github.com>
Co-authored-by: jruales <1588988+jruales@users.noreply.github.com>
Co-authored-by: jruales <1588988+jruales@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
|
@copilot also update the PR description to reflect the latest changes |
There was a problem hiding this comment.
Pull request overview
This PR adds configurable Integrated Browser routing for non-localhost HTTP(S) links, alongside the existing localhost/all-interfaces behavior.
Changes:
- Adds
workbench.browser.openExternalLinks. - Extracts URL-to-setting classification into a shared helper with tests.
- Updates opener telemetry/source naming for external-link opens.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/contrib/browserView/common/browserLinkOpeners.ts |
Adds helper for classifying HTTP(S) links to the relevant opener setting. |
src/vs/workbench/contrib/browserView/test/common/browserLinkOpeners.test.ts |
Adds unit coverage for localhost, external, and invalid/non-HTTP(S) links. |
src/vs/workbench/contrib/browserView/electron-browser/features/browserTabManagementFeatures.ts |
Uses the helper in the opener contribution and registers the new setting. |
src/vs/platform/browserView/common/browserViewTelemetry.ts |
Adds telemetry source for external-link opener usage. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Co-authored-by: jruales <1588988+jruales@users.noreply.github.com>
Resolved in |
Updated the PR description to reflect the latest state of the branch, including the setting-key classification refactor, caller-side config lookup, and the merge-resolution result in |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: jruales <1588988+jruales@users.noreply.github.com>
| ), | ||
| agentsWindow: { default: true }, | ||
| }, | ||
| 'workbench.browser.openExternalLinks': { |
There was a problem hiding this comment.
I think I'd prefer to have a list of rules for which links to open or not -- some things like GitHub might stay better in the browser while e.g docs should be opened in VS Code. I'm thinking something like gitignore syntax but for URLs, or a list of items with a pattern and dropdown for whether it should be opened internally or externally.
| return false; | ||
| } | ||
| } catch { | ||
| const openerSetting = getIntegratedBrowserLinkOpenerSetting(href); |
There was a problem hiding this comment.
It seems weird to get the configuration key in a helper, but then test the actual value here? Might make more sense to have a shouldOpenInIntegratedBrowser helper that just takes the URL and config service. I also question the value of having a whole new file for this one helper that is only used here.
This change adds a dedicated setting to route non-localhost HTTP(S) links into the Integrated Browser, complementing the existing localhost-only behavior. Link routing now selects the appropriate setting based on URL authority instead of hardcoding localhost checks in the opener contribution.
Configuration
workbench.browser.openExternalLinksto open non-localhost HTTP(S) links in the Integrated Browser.workbench.browser.openLocalhostLinkssemantics unchanged for localhost/all-interfaces targets.Link opener behavior
browserLinkOpeners.tsso opener decisions are centralized.externalLinkOpenervslocalhostLinkOpener).Structure and naming
BrowserLinkOpenerContributionwith IDworkbench.contrib.browserLinkOpenerto reflect expanded scope.Targeted coverage