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

Links/URLs get offset when certain unicode characters are printed #8709

Closed
Entrivax opened this issue Jan 5, 2021 · 13 comments · Fixed by #11915
Closed

Links/URLs get offset when certain unicode characters are printed #8709

Entrivax opened this issue Jan 5, 2021 · 13 comments · Fixed by #11915
Assignees
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@Entrivax
Copy link

Entrivax commented Jan 5, 2021

Environment

Windows build number: Windows 10 version 2004 (10.0.19041.0)
Windows Terminal version: Preview 1.5.3242.0

Steps to reproduce

Print certain unicode characters like this one: 𝓐

Expected behavior

Links/URLs should be valid.

Actual behavior

Links/URLs get offset and break with the presence of certain unicode characters.
For example, if the following text is printed:

𝓐𝓐𝓐
https://microsoft.com/

image

The hoverable link, clickable link and opened link is ps://microsoft.com/
If those unicode characters are not printed (if you scroll for example), the links work again.

2021-01-05_22-44-13_kWeoQrwJie.mp4

Note that the emoji doesn't cause the issue.
The code used in the video is:

console.log(`𝓐𝓐𝓐𝓐𝓐𝓐 123 https://microsoft.com/

✨ https://google.com/
https://google.fr/
https://youtube.com/
https://aka.ms/`)
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jan 5, 2021
@zadjii-msft
Copy link
Member

Oh weird. I would have thought this was a dupe of #8121, but that one was fixed in 1.5.3142, so this must be it's own variation on the subject.

Those 𝓐 characters are really messing it up, huh.

@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Jan 6, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 6, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Jan 6, 2021
@Entrivax
Copy link
Author

Entrivax commented Jan 6, 2021

It looks a lot like it, going the other way from the screen of that issue.

Those 𝓐 characters are really messing it up, huh.

Yeah, I found the problem when printing display usernames from an api. Not only the 𝓐 character, but also the rest of the alphabet written like that. Maybe there is more characters than that, I didn't try to find them all.

@DHowett DHowett self-assigned this Jan 7, 2021
@skyline75489
Copy link
Collaborator

The 𝓐 is considered as two wide chars because of #8124. Another place we might want to use a comprehensive Unicode width estimator .

@DHowett DHowett mentioned this issue Jan 12, 2021
15 tasks
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 22, 2021
@zed76r
Copy link

zed76r commented May 10, 2021

image

image

@mas90
Copy link

mas90 commented Jul 22, 2021

Yeah, I found the problem when printing display usernames from an api. Not only the 𝓐 character, but also the rest of the alphabet written like that. Maybe there is more characters than that, I didn't try to find them all.

Definitely more characters than that, e.g. 🎟.

echo -e '\xf0\x9f\x8e\x9f http://www.example.com/' reproduces the bug on version 1.9.1445.0.

@ghost ghost added the In-PR This issue has a related PR label Dec 9, 2021
@ghost ghost closed this as completed in #11915 Dec 9, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Dec 9, 2021
ghost pushed a commit that referenced this issue Dec 9, 2021
)

<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request

Properly handle UTF-16 surrogates when calculating the position of matched pattern.

Fix #8709

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> 
## References
https://github.com/microsoft/terminal/blob/b88ffb21b0725331877ba76bac5a79a4c21eaa03/src/buffer/out/search.cpp#L335-L339

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #8709
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
use `Utf16Parser::Parse` to handle code points from U+010000 to U+10FFFF in UTF-16.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

![image](https://user-images.githubusercontent.com/1068203/145421736-c842c7d4-0136-42d0-ad72-f004f58d9e3b.png)

also the case by @mas90  #8709 (comment):

![image](https://user-images.githubusercontent.com/1068203/145420264-3fe220b4-42c5-44ac-aa94-4e604b164ed3.png)
DHowett pushed a commit that referenced this issue Dec 13, 2021
)

<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request

Properly handle UTF-16 surrogates when calculating the position of matched pattern.

Fix #8709

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
https://github.com/microsoft/terminal/blob/b88ffb21b0725331877ba76bac5a79a4c21eaa03/src/buffer/out/search.cpp#L335-L339

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #8709
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
use `Utf16Parser::Parse` to handle code points from U+010000 to U+10FFFF in UTF-16.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

![image](https://user-images.githubusercontent.com/1068203/145421736-c842c7d4-0136-42d0-ad72-f004f58d9e3b.png)

also the case by @mas90  #8709 (comment):

![image](https://user-images.githubusercontent.com/1068203/145420264-3fe220b4-42c5-44ac-aa94-4e604b164ed3.png)

(cherry picked from commit a2d96d6)
DHowett pushed a commit that referenced this issue Dec 13, 2021
)

<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request

Properly handle UTF-16 surrogates when calculating the position of matched pattern.

Fix #8709

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
https://github.com/microsoft/terminal/blob/b88ffb21b0725331877ba76bac5a79a4c21eaa03/src/buffer/out/search.cpp#L335-L339

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [ ] Closes #8709
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
use `Utf16Parser::Parse` to handle code points from U+010000 to U+10FFFF in UTF-16.

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

![image](https://user-images.githubusercontent.com/1068203/145421736-c842c7d4-0136-42d0-ad72-f004f58d9e3b.png)

also the case by @mas90  #8709 (comment):

![image](https://user-images.githubusercontent.com/1068203/145420264-3fe220b4-42c5-44ac-aa94-4e604b164ed3.png)

(cherry picked from commit a2d96d6)
@DHowett
Copy link
Member

DHowett commented Dec 14, 2021

Clever quick fix -- thanks @comzyh! I'm rolling this into the 1.11 and 1.12 servicing releases we're putting out to make Terminal more robust over the holidays. :)

@ghost
Copy link

ghost commented Dec 14, 2021

🎉This issue was addressed in #11915, which has now been successfully released as Windows Terminal Preview v1.12.3472.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Dec 14, 2021

🎉This issue was addressed in #11915, which has now been successfully released as Windows Terminal v1.11.3471.0.:tada:

Handy links:

@CerebralDatabank
Copy link

I'm still having this issue with 5-digit Nerd Font Unicode characters on Windows 11…

image

@DHowett
Copy link
Member

DHowett commented Mar 28, 2024

Which version of Terminal are you using? This issue was closed two years ago 🙂

@j4james
Copy link
Collaborator

j4james commented Mar 28, 2024

@DHowett I think this regressed recently, because I was able to reproduce the issue until I got the update from 3 days ago. I'm assuming it was fixed by this:

Auto-detected URLs and search results are no longer pushed around by wide characters (oops) (#16787)

@CerebralDatabank Bottom line is you probably just need to get the very latest update.

@CerebralDatabank
Copy link

@DHowett Haha I knew that, but what can I say, I still have the issue 😅 thought replying here was better than opening a duplicate lol

@j4james I think you're right, latest stable is v1.19.10821.0 and I have v1.19.10573.0. But nothing happens when I check for updates in Terminal and there's no "Update" in the Store page.

If I install the latest release via the .msixbundle from here, will that mess up the existing link with MS Store updates? Or should I just wait until it rolls out to my device?

@DHowett
Copy link
Member

DHowett commented Mar 28, 2024

thought replying here was better

you know what, that's totally fair! hah!

will that mess up the ... link

Nope, you can install the msixbundle from here and it will still hook up to the store for future updates!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants