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

Web links not suport with port numbers #14260

Closed
1 task
hacku5 opened this issue Nov 3, 2021 · 9 comments
Closed
1 task

Web links not suport with port numbers #14260

hacku5 opened this issue Nov 3, 2021 · 9 comments
Labels
Cost-Small Small work item - 0-8 hours of work Issue-Bug Something isn't working Priority-2 Bug that is medium priority Product-PowerToys Run Improved app launch PT Run (Win+R) Window Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Things that relate with PowerToys Run's plugin interface

Comments

@hacku5
Copy link

hacku5 commented Nov 3, 2021 •
edited
Loading

Microsoft PowerToys version

0.33.1 0.49.1 =<

Running as admin

  • Yes

Area(s) with issue?

PowerToys Run

Steps to reproduce

https://imgur.com/a/Xq5ODCi/

✔️ Expected Behavior

Link's Opening with default web browser

❌ Actual Behavior

it asks which application to open

Other Software

No response
@hacku5 hacku5 added Issue-Bug Something isn't working Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Nov 3, 2021
@Ahsoka
Copy link

Ahsoka commented Nov 3, 2021

When you click your link, https://imgur.com/a/Xq5ODCi, it redirects to: https://github.com/microsoft/PowerToys/issues/url instead of the actual image.

@crutkas
Copy link
Member

crutkas commented Nov 3, 2021

if you do foo.com:80 it throws up an "find app" dialog.

Wonder if this is a Win11 regression, thought this worked

@crutkas
Copy link
Member

crutkas commented Nov 3, 2021

verified with 0.49.1

@crutkas crutkas added Priority-2 Bug that is medium priority Product-PowerToys Run Improved app launch PT Run (Win+R) Window Run-Plugin Things that relate with PowerToys Run's plugin interface Cost-Small Small work item - 0-8 hours of work Help Wanted We encourage anyone to jump in on these and submit a PR. Good first issue Good for newcomers. and removed Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Nov 3, 2021
@franky920920
Copy link
Contributor

@crutkas In this case, I believe Windows think the foo.com is the application scheme and the 80 is the address (parameters to the application). I think this will be hard to fix after the supports of the application URI is fixed.

@htcfreek
Copy link
Collaborator

htcfreek commented Nov 4, 2021

why not showing two results in this case? One for browser and one for app?

@franky920920 franky920920 mentioned this issue Nov 16, 2021
Merged
10 tasks
@ChaseKnowlden
Copy link
Contributor

Still happening with PT 0.58.0

@jaimecbernardo jaimecbernardo added this to the Help Wanted milestone May 16, 2022
@KohGeek
Copy link
Contributor

KohGeek commented Aug 6, 2022 •
edited
Loading

I'm currently trying to figure out this issue.
It seems to be a problem with UrlBuilder (or maybe it's a feature?). For anything that is formatted in URL:port, URLBuilder treats the URL part as schema, and the port part as the path/host.
image
This could be resolved with a few more checks, is it a correct fix to isolate this pattern, or is "tel:411" a valid scheme/path URI?

@KohGeek
Copy link
Contributor

KohGeek commented Aug 6, 2022

Linking dotnet/runtime#73521 as the issue raised with the dotnet team

@KohGeek
Copy link
Contributor

KohGeek commented Aug 7, 2022

@crutkas @jaimecbernardo considering on the UriBuilder side, they confirmed this is intended behaviour, do we just assume otherwise for this use case?

KohGeek added a commit to KohGeek/PowerToys that referenced this issue Aug 7, 2022
@KohGeek
[PT Run] Fix web link with ports support (microsoft#14260)
b58d933 
* URL in the format of `domain:port` now directs to default browser

* Add tests to verify web link with ports scenario

* Fix test case and scenario where mismatching schema and port for IPv6 does not result in correct output
@KohGeek KohGeek mentioned this issue Aug 7, 2022
Merged
11 tasks
jaimecbernardo pushed a commit that referenced this issue Aug 11, 2022
@KohGeek
[PTRun][URL]Fix web link with ports support (#19809)
efa0918 
* [PT Run] Fix web link with ports support (#14260)

* URL in the format of `domain:port` now directs to default browser

* Add tests to verify web link with ports scenario

* Fix test case and scenario where mismatching schema and port for IPv6 does not result in correct output

* [PT Run][Tests] Change and add more UriParser Tests

* Specifically of note is line 56, where [IPv6]:80 diverts to https instead of http.

* [PT Run][Tests] Add UriParser tests

* Add more tests targeting port handling

* [PT Run] Fix http handling

* This also fixes oddity with IPv4 and IPv6 handling

* [PT Run] Add second results depending on condition

* Test: update all test to reflect updated functions & add a little more tests

* Update function to show two results when URI is in the format of `domain:port` (situation where it can also be `schema:path`)

* Update regex style to follow previous code

* [PT Run] Change tests and filter localhost from certain results

* Add tests for 127.0.0.1, localhost, and ::1

* Move test around into more logical arrangement

* Filter localhost out from showing double results

* [PT Run] Fix spelling on comments

* [PT Run] Add some words to expect.txt

* [PT Toys] Clarify comment regarding [::]

Co-authored-by: Heiko <61519853+htcfreek@users.noreply.github.com>

* [PT Run] Remove tests regarding tel protocol

* [PT Run] Clarify UriParser parameter

* [PT Run] Add UriParser tests for tel protocol

* Current code has a regression bug where tel:xxxx, if xxxx is more than 65536 it will break. Will fix in follow up commit.

* [PT Run] Refactor ExtendedUriParser and its tests

* Remove `isWebUri` from ExtendedUriParser, keeping only webUri and systemUri

* Tel protocol regression bug still exists

* [PT Run] Fix wrong icon when webUri result

* [PT Run] Fix regression bug for tel protocol

* Tel protocol will sometimes bug out when tel:xxxx if xxxxx is more than 65535, as UriBuilder will throw error thinking the port number has been exceeded

* [PT Toy] Fix tel test

* [PT Run] Changes to tests

* Add test for application uri to include ports, for all non-protocol, http and https variants

* Rearrange some more test to make more logical sense, and add comments

* [PT Run] Simplify code

* Move webUri and systemUri to be global, as per htcfreek's recommendation

* Add comment to empty catch

* Change null to default

* [PT Toy] Update test name

Co-authored-by: Heiko <61519853+htcfreek@users.noreply.github.com>

* [PT Toy] Change result prompt when empty string

* [PT Toy] Fix typo in comment

* [PT Toy] Simplify line

* [PT Toy] Change result prompt when empty string
@jaimecbernardo jaimecbernardo added the Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. label Aug 11, 2022
@Jay-o-Way Jay-o-Way removed Help Wanted We encourage anyone to jump in on these and submit a PR. Good first issue Good for newcomers. labels Aug 11, 2022
@Jay-o-Way Jay-o-Way removed this from the Help Wanted milestone Aug 11, 2022
@crutkas crutkas closed this as completed Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Cost-Small Small work item - 0-8 hours of work Issue-Bug Something isn't working Priority-2 Bug that is medium priority Product-PowerToys Run Improved app launch PT Run (Win+R) Window Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Things that relate with PowerToys Run's plugin interface
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@crutkas @KohGeek @franky920920 @jaimecbernardo @Ahsoka @ChaseKnowlden @htcfreek @hacku5 @Jay-o-Way