Skip to content

cache OpenURLOptions in ExtensionUrlBootstrapHandler#110725

Merged
joaomoreno merged 1 commit intomicrosoft:masterfrom
LeuisKen:master
Nov 23, 2020
Merged

cache OpenURLOptions in ExtensionUrlBootstrapHandler#110725
joaomoreno merged 1 commit intomicrosoft:masterfrom
LeuisKen:master

Conversation

@LeuisKen
Copy link
Contributor

@LeuisKen LeuisKen commented Nov 16, 2020

The ExtensionUrlBootstrapHandler should keep options of method handleURL in cache.

rheh
rheh reviewed Nov 16, 2020
View reviewed changes
Copy link

@rheh rheh left a comment

Choose a reason for hiding this comment

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

LGMT.

@joaomoreno
Copy link
Member

joaomoreno commented Nov 17, 2020

Does this fix an actual issue?

@joaomoreno joaomoreno added the info-needed Issue requires more information from poster label Nov 17, 2020
@LeuisKen
Copy link
Contributor Author

LeuisKen commented Nov 18, 2020

The OpenUrlAction passing the { trusted: true } options to the urlService, but when matching ExtensionUrlBootstrapHandler the options will lose.

@joaomoreno
Copy link
Member

joaomoreno commented Nov 18, 2020

Again: what bug in VS Code does this actually fix?

@LeuisKen
Copy link
Contributor Author

LeuisKen commented Nov 18, 2020

Here is the reproduce screenshot:

ezgif-7-c575d3262b01

  1. Open VS Code Web.
  2. Ctrl + shift + p to show all commands.
  3. Using Open URL command, type vscode://vscode.git/clone URL and waiting for the extensionHost start.
  4. See that VS Code warning us that it will open the vscode://vscode.git/clone URL, just cancel it.
  5. Using Open URL command again and type vscode://vscode.git/clone URL.
  6. The URL open directly without any warning.

@joaomoreno
Copy link
Member

joaomoreno commented Nov 19, 2020

@LeuisKen Thanks for that. I suggest next time to file the issue first. I've created it for you: #110953

@joaomoreno joaomoreno removed the info-needed Issue requires more information from poster label Nov 19, 2020
@bpasero bpasero added the info-needed Issue requires more information from poster label Nov 20, 2020
@bpasero
Copy link
Member

bpasero commented Nov 20, 2020

I do not understand what the issue is nor what this PR fixes.

@bpasero
Copy link
Member

bpasero commented Nov 21, 2020

Unassigning myself as it changes code I did not write. It came in via 682cb31

@bpasero bpasero assigned joaomoreno and bpasero and unassigned joaomoreno and bpasero Nov 21, 2020
@joaomoreno joaomoreno removed the info-needed Issue requires more information from poster label Nov 23, 2020
@joaomoreno joaomoreno added this to the November 2020 milestone Nov 23, 2020
@joaomoreno joaomoreno merged commit ee6e905 into microsoft:master Nov 23, 2020
@joaomoreno
Copy link
Member

joaomoreno commented Nov 23, 2020

Thanks! 🍻

@joaomoreno joaomoreno mentioned this pull request Nov 23, 2020
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@rheh rheh rheh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@joaomoreno joaomoreno

Labels

None yet

Projects

None yet

Milestone

November 2020

Development

Successfully merging this pull request may close these issues.

4 participants

@LeuisKen @joaomoreno @bpasero @rheh