Skip to content

Don't open two browser tabs when clicking the pull request number - #8962

Open
Lukas (L4XB) wants to merge 1 commit into
microsoft:mainfrom
L4XB:fix/8955-pr-number-opens-two-tabs
Open

Lukas (L4XB) wants to merge 1 commit into
microsoft:mainfrom
L4XB:fix/8955-pr-number-opens-two-tabs

Conversation

@L4XB

Copy link
Copy Markdown

Fixes #8955

Cause

Not #8919, which the issue guesses at — that PR does not touch link handling. The
regression is 02ac4de ("Fix various things in the webview that don't work correctly in
Agents window", #8935), which fixed #8934 by giving the #<number> links in
webviews/components/header.tsx and webviews/components/stickyHeader.tsx an onClick:

onClick={event => {
	event.preventDefault();
	void openOnGitHub();
}}

Both anchors keep their href, and the webview host opens any anchor with an href on its
own. handleInnerClick in
src/vs/workbench/contrib/webview/browser/pre/index.html (microsoft/vscode) is registered
with contentWindow.addEventListener('click', handleInnerClick) and posts did-click-link
for the first anchor in the click's composed path. It does not check
event.defaultPrevented — the contextmenu handler registered a few lines below it does.
React 16 delegates onClick at document, so preventDefault() runs and the event then
carries on bubbling to the content window, where the host handler runs regardless.

One click therefore produces two opens:

  • did-click-linkIOpenerService.open
  • pr.openOnGitHubopenItemOnGitHub (src/commands.ts) → openWithDefaultExternalOpener

With githubPullRequests.openPullLinks off — its default since #8942 — nothing claims the
first one, so both reach the browser: the two tabs in the report. With the setting on, the
extension's own external URI opener claims the did-click-link and re-reveals the overview
instead, so the duplicate is less visible but still there.

These two anchors are the only ones under webviews/ that carry both an href and an
onClick; every other <a onClick> in the tree has no href, which is why they were never
affected.

Fix

Stop the click from reaching the host, so exactly one code path opens the URL.
webviews/editorWebview/app.tsx already does this for code reference links, with a
capture-phase listener that calls preventDefault() and stopPropagation().

Keeping the href preserves the hover target, keyboard activation and the copy-link context
menu; dropping the onClick instead would bring #8934 back, so the Agents-window path stays
exactly as #8935 left it.

Arguably the deeper defect is that handleInnerClick should honour defaultPrevented the
way the neighbouring contextmenu handler does. That lives in microsoft/vscode, so this
change does not wait on it.

Test

webviews/editorWebview/test/overview.test.tsx gains opens a PR number link exactly once.
It registers the host's behaviour on window — open any anchor with an href that the click
reaches, without checking defaultPrevented — clicks each number link and asserts one
openOnGitHub call and no host open, i.e. exactly one external open per click.

At the merge base (2f06150), with only the test applied:

  Overview
    ✔ renders the PR header with title (43ms)
    ✔ opens PR number links on GitHub
    1) opens a PR number link exactly once
    ✔ shows view changes in both headers
    ✔ does not show view changes outside the agents window
    ✔ applies sticky class when scrolled
    2) applies deferred pull request updates

  5 passing (102ms)
  2 failing

  1) Overview
       opens a PR number link exactly once:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ [
+   'https://github.com/owner/name/pulls/1234'
+ ]
- []

With the fix:

  Overview
    ✔ renders the PR header with title (44ms)
    ✔ opens PR number links on GitHub
    ✔ opens a PR number link exactly once
    ✔ shows view changes in both headers
    ✔ does not show view changes outside the agents window
    ✔ applies sticky class when scrolled
    1) applies deferred pull request updates

  6 passing (101ms)
  1 failing

applies deferred pull request updates fails the same way on both sides and is unrelated to
this change: PRContext.updatePR calls updateState, which does
Object.assign(vscode.getState(), data), and the test constructs new PRContext(pr) without
anything ever calling setState, so getState() is undefined and it throws
TypeError: Cannot convert undefined or null to object.

These tests are not currently executed anywhere

webviews/**/test/*.test.tsx is type-checked through tsconfig.webviews.json, but nothing
runs it. tsconfig.test.json includes only src/@types, src/common, src/github and
src/test; src/test/index.ts loads require.context('./', …); and after npm run compile
no webview test appears in dist/ — neither the new test nor the existing
applies sticky class when scrolled. That also explains how the failure above went
unnoticed.

I have left the test next to the code it covers, with the three sibling webview test files,
rather than growing this fix into a test-infrastructure change — turning the directory on
today would redden CI on pre-existing failures in overview.test.tsx and app.test.tsx.
Happy to split that out into its own PR if you want it.

I ran the file with mocha and ts-node against tsconfig.webviews.json (module: commonjs),
with a setup file that installs jsdom-global plus requestAnimationFrame, matchMedia and
IntersectionObserver stubs and an acquireVsCodeApi mirroring
src/test/mocks/mockWebviewEnvironment.ts, and require.extensions hooks standing in for the
svg-inline-loader and CSS webpack loaders.

Validation

  • npm run compile — success (extension:node, extension:webworker, webviews)
  • npm run lint — exit 0, no files changed
  • npm run hygiene — exit 0
  • npm run check:commands — all declared commands are registered
  • npm test — 559 passing, 0 failing (test:scripts 75 passing, extension host suite exit 0)
  • npx tsc --noEmit for tsconfig.json, tsconfig.webviews.json and tsconfig.test.json — all clean

File and line references are against main @ 2f06150a, and microsoft/vscode main @
f80869ac.

02ac4de gave the pull request number links in the overview header and the
sticky header an onClick that calls preventDefault and opens the item on
GitHub, so that the link also works in the Agents window. The webview host
opens any anchor with an href as well, and its click handler does not check
defaultPrevented, so in a regular window the click is handled twice and the
browser gets two tabs for the same URL.

Stop the click from reaching the host, the way the code reference link handler
in webviews/editorWebview/app.tsx already does.

Fixes microsoft#8955
Copilot AI lite review requested due to automatic review settings September 16, 2026 12:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

clicking PR number opens two github tabs since 0.166.0 Pressing on the PR link in the agents window doesn't open the browser

2 participants