Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: protocol handler opens a new window and not existing window #142685

Closed
willwad opened this issue Feb 9, 2022 · 11 comments
Closed

Windows: protocol handler opens a new window and not existing window #142685

willwad opened this issue Feb 9, 2022 · 11 comments
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron electron-16-update Issues related to electron 16 update regression Something that used to work is now broken upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream verification-steps-needed Steps to verify are needed for verification verified Verification succeeded windows VS Code on Windows issues workbench-os-integration Native OS integration issues
Milestone

Comments

@willwad
Copy link

willwad commented Feb 9, 2022

Issue Type: Bug

With this release, invocations of vscode-insiders:// links always result in a new instance of VS Code opening, and vscode.window.registerUriHandler registrations in the new instance never getting called.

I also see an error of:
Warning: 'open-url --' is not in the list of known options, but still passed to Electron/Chromium.

when doing system invocations of vscode-insiders://

This breaks the extension I own as it uses the URI hanlder to notify the extension from cmdlet actions in the terminal.

VS Code version: Code - Insiders 1.65.0-insider (d09aeab, 2022-02-09T05:16:17.063Z)
OS version: Windows_NT x64 10.0.22000
Restricted Mode: No

System Info
Item Value
CPUs Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz (12 x 3696)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 31.78GB (8.42GB free)
Process Argv --crash-reporter-id eca4b774-c277-477f-863a-1ad060bb6a90
Screen Reader no
VM 0%
Extensions (16)
Extension Author (truncated) Version
vscode-research alb 201021.0.1
npm-intellisense chr 1.4.0
vscode-markdownlint Dav 0.46.0
vscode-eslint dba 2.2.2
vscode-npm-script eg2 0.3.24
vscode-drawio hed 1.6.4
tdpcode Mic 1.10.9
remote-ssh ms- 0.70.0
remote-ssh-edit ms- 0.70.0
remote-wsl ms- 0.64.2
vscode-remote-extensionpack ms- 0.21.0
cpptools ms- 1.8.4
js-debug-companion ms- 1.0.15
powershell-preview ms- 2022.2.0
debugger-for-chrome msj 4.13.0
code-spell-checker str 2.1.6
A/B Experiments
vsliv695:30137379
vsins829:30139715
vsliv368cf:30146710
vsreu685:30147344
python383cf:30185419
vspor879:30202332
vspor708:30202333
vspor363:30204092
pythonvspyl392:30422396
pythontb:30258533
pythonvspyt551:30291412
pythonptprofiler:30281269
vsdfh931:30280409
vshan820:30294714
pythondataviewer:30285072
vscod805:30301674
pythonvspyt200:30323110
bridge0708:30335490
bridge0723:30353136
vsaa593cf:30376535
pythonvs932:30404738
wslgetstarted:30421357
vs360cf:30404996
vsclayoutctrc:30405799
pythonvsnew555:30426298
vscscmwlcmt:30428973
azactmsalcf:30432849
vscgsvid2:30433758

@yume-chan
Copy link
Contributor

Related to #142666

@bpasero bpasero assigned joaomoreno, bpasero and deepak1556 and unassigned Tyriar Feb 10, 2022
@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug electron-16-update Issues related to electron 16 update regression Something that used to work is now broken and removed triage-needed labels Feb 10, 2022
@bpasero bpasero added this to the February 2022 milestone Feb 10, 2022
@bpasero bpasero added the windows VS Code on Windows issues label Feb 10, 2022
@bpasero
Copy link
Member

bpasero commented Feb 10, 2022

I can only reproduce since we use Electron 16, so I suspect it is related. And it only reproduces on Windows for me.

I am not so familiar with how we setup the protocol handler, code seems to be here:

app.setAsDefaultProtocolClient(productService.urlProtocol, process.execPath, windowsParameters);

Maybe we no longer pass the arguments properly to the instance? Could this be related to the request single instance lock changes from @rzhao271 ?

@bpasero bpasero added the workbench-os-integration Native OS integration issues label Feb 10, 2022
@bpasero bpasero changed the title The uri handler does not work on this insiders release Windows: protocol handler opens a new window and not existing window Feb 10, 2022
@deepak1556
Copy link
Contributor

request single instance lock exercises different code path than the protocol handler, so that change definitely shouldn't have an impact here.

Warning: 'open-url --' is not in the list of known options, but still passed to Electron/Chromium.

Considering this error, looks like the arg is incorrectly parsed. Not sure if its the runtime that is returning the bad argument, will check.

@bpasero
Copy link
Member

bpasero commented Feb 10, 2022

I would expect this place to be the location where we handle the URI:

// Check for URIs to open in window
const windowOpenableFromProtocolLink = app.getWindowOpenableFromProtocolLink(uri);
if (windowOpenableFromProtocolLink) {
const [window] = windowsMainService.open({
context: OpenContext.API,
cli: { ...environmentService.args },
urisToOpen: [windowOpenableFromProtocolLink],
gotoLineMode: true
// remoteAuthority: will be determined based on windowOpenableFromProtocolLink
});
window.focus(); // this should help ensuring that the right window gets focus when multiple are opened
return true;
}

I added some logging at that place.

@bpasero
Copy link
Member

bpasero commented Feb 10, 2022

Probably same as electron/electron#32463 which was reported against Electron.

@bpasero bpasero added electron Issues and items related to Electron upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Feb 10, 2022
mustard-mh pushed a commit to gitpod-io/openvscode-server that referenced this issue Feb 13, 2022
mustard-mh pushed a commit to gitpod-io/openvscode-server that referenced this issue Feb 13, 2022
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Feb 16, 2022

This issue affects GitHub and Microsoft authentication flows, unfortunately so users will not be able to sign in at all until this is sorted. Just wanted to leave this context here as I will point others to this issue.

@IllusionMH
Copy link
Contributor

Is MS Authenticator affected? I think it's "self hosted" server, so no app schema handlers should be triggered.

@TylerLeonhardt
Copy link
Member

True, @IllusionMH. Using the protocol handler is only done in Microsoft Auth if spinning up that server you mention doesn't work which should be relatively rare... so in auth world this mostly affects GitHub auth. Unfortunately, GitHub auth is used more than Microsoft auth.

@IllusionMH
Copy link
Contributor

Oh, nice to know. I stand corrected.

@deepak1556
Copy link
Contributor

Will be fixed in today's insider

Refs https://domoreexp.visualstudio.com/Teamspace/_git/electron-build/pullrequest/479313

@deepak1556 deepak1556 added the author-verification-requested Issues potentially verifiable by issue author label Feb 18, 2022
@TylerLeonhardt
Copy link
Member

I verified that this isn't happening anymore, however, since it was related to the electron update and we've deferred that to March, I will move this out to March for true verification.

@roblourens roblourens added the verification-steps-needed Steps to verify are needed for verification label Mar 24, 2022
@joaomoreno joaomoreno added the verified Verification succeeded label Mar 25, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron electron-16-update Issues related to electron 16 update regression Something that used to work is now broken upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream verification-steps-needed Steps to verify are needed for verification verified Verification succeeded windows VS Code on Windows issues workbench-os-integration Native OS integration issues
Projects
None yet
Development

No branches or pull requests

10 participants