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.

Sign up for GitHub

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

Jump to bottom

Integrated terminal, wrong URL encoding for clickable links #144898

Closed
PapsOu opened this issue Mar 11, 2022 · 5 comments · Fixed by #146202
Closed

Integrated terminal, wrong URL encoding for clickable links #144898

PapsOu opened this issue Mar 11, 2022 · 5 comments · Fixed by #146202
Assignees
@Tyriar
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-links verified Verification succeeded
Milestone
April 2022

Comments

@PapsOu
Copy link

PapsOu commented Mar 11, 2022

Does this issue occur when all extensions are disabled?: Yes

The behavior was correct few weeks ago.

Version : 1.65.0
Validation : b5205cc
Date : 2022-03-02T11:12:36.248Z
Electron : 13.5.2
Chromium: 91.0.4472.164
Node.js : 14.16.0
V8 : 9.1.269.39-electron.0
Système d’exploitation : Linux x64 5.16.11-100.fc34.x86_64

Steps to Reproduce:

  1. open an integrated terminal and type the following command : echo 'http://localhost/?t=toto%2Btiti'
  2. then, alt + click on the printed link
  3. your browser is open with url http://localhost/?t=toto+titi, but it should be opened with url http://localhost/?t=toto%2Btiti

Peek 11-03-2022 10-17

Working test on native terminal :

  1. open a native linux terminal (for my DE, it is gnome-terminal with zsh)
  2. type the following command : echo 'http://localhost/?t=toto%2Btiti'
  3. then, ctrl + click on the printed link
  4. your browser is open with url http://localhost/?t=toto%2Btiti, as this is the correct expected behavior

Peek 11-03-2022 10-18
@meganrogge
Copy link
Contributor

Looks like we're passing the right thing to the opener service @jrieken @JacksonKearl
Screen Shot 2022-03-11 at 10 45 32 AM

vscode/src/vs/workbench/contrib/terminal/browser/links/terminalLinkOpeners.ts

Lines 212 to 215 in 0e8fceb

this._openerService.open(link.uri || URI.parse(link.text), {
allowTunneling: this._isRemote,
allowContributedOpeners: true,
});

@jongio
Copy link

jongio commented Mar 25, 2022

I can repro this with:

print("https://portal.azure.com/#blade/HubsExtension/DeploymentDetailsBlade/overview/id/%2Fsubscriptions%2F{{ - subId - }}%2Fproviders%2FMicrosoft.Resources%2Fdeployments%2F{{ - deployment name - }}")

I can copy and paste that URL into a browser and it will work.
I can run the same code in regular terminal and it will work.

It doesn't work when I click on the link from w/in the vscode terminal.

@Tyriar
Copy link
Member

Tyriar commented Mar 28, 2022

This has come up before where it was closed by design, originally I don't think it was possible to open a link like this because opener service always works on uris and they always encode like this. I notice though that the editor properly retains the %2B in a link though so it should be possible now.

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal-links labels Mar 28, 2022
@Tyriar Tyriar modified the milestones: Backlog, April 2022 Mar 28, 2022
@meganrogge meganrogge removed their assignment Mar 28, 2022
@jongio
Copy link

jongio commented Mar 28, 2022

Okay thanks!

Tyriar added a commit that referenced this issue Mar 28, 2022
@Tyriar
Avoid converting pre-encoded values in terminal URL links
27545b8 
Fixes #144898
@Tyriar Tyriar mentioned this issue Mar 28, 2022
Merged
@jongio
Copy link

jongio commented Mar 28, 2022

Thank you @Tyriar

@jrieken jrieken added the verified Verification succeeded label Apr 28, 2022
@LucaMueller1 LucaMueller1 mentioned this issue May 2, 2022
Closed
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-links verified Verification succeeded
Projects
None yet
Milestone
April 2022
Development

Successfully merging a pull request may close this issue.

Avoid converting pre-encoded values in terminal URL links microsoft/vscode
6 participants
@jrieken @jongio @Tyriar @PapsOu @meganrogge and others