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 get wrong position when they are after wide chars with canvas/webgl renderer #4294

Closed
JasonXJ opened this issue Dec 7, 2022 · 5 comments · Fixed by #4288
Closed

Links get wrong position when they are after wide chars with canvas/webgl renderer #4294

JasonXJ opened this issue Dec 7, 2022 · 5 comments · Fixed by #4288
Milestone

Comments

@JasonXJ
Copy link

JasonXJ commented Dec 7, 2022

See the image below. Note that it only happens to the links detected by the weblinks addon but not OSC 8 links. It also only happens to webgl and canvas renderers but the dom one.
image

I looked at the code a bit. It seems that the web link addon uses the character position (i.e. "哈" is counted 1 even though it is a wide character) as the the value for IBufferCellPosition.x. Code here. Is this the correct way?

BTW, I am not sure whether this is just #2887.

Details

  • Browser and browser version: Chrome 110
  • xterm.js version: Tot
@jerch
Copy link
Member

jerch commented Dec 7, 2022

The weblink addon is broken in several ways, which is basically the reason for #4288. Maybe check if the issue remains with that PR?

@JasonXJ
Copy link
Author

JasonXJ commented Dec 8, 2022

Hi Jerch, your PR works great! It not only fixes this issue, but also 1) works for special shortlink such as http:/b/123456; 2) no longer includes trailing . in the url so that urls in documentation et al. are not messed up.

Can I expect this to be merged soon?

@JasonXJ
Copy link
Author

JasonXJ commented Dec 8, 2022

There is still some rendering issue with the DOM renderer (I care much more about the webgl renderer though). See the screenshot below --- my mouse cursor is on the link in the first row.

image

@jerch
Copy link
Member

jerch commented Dec 8, 2022

Can I expect this to be merged soon?

Well it still needs some more test cases, I kinda turned the whole regexp upside-down to also match unicode chars in paths (nowadays supported by all browsers), but this makes match exclusions a bit more cumbersome.

There is still some rendering issue with the DOM renderer ...

Had to fix the DOM renderer offset calc (was totally off with correct cell positions), seems there is still a one off issue at the end of underline.

jerch added a commit to jerch/xterm.js that referenced this issue Dec 8, 2022
@jerch
Copy link
Member

jerch commented Dec 8, 2022

@JasonXJ Should work now (plz report in #4288, if something is still totally off 🤞)

@jerch jerch mentioned this issue Dec 8, 2022
4 tasks
@Tyriar Tyriar added this to the 5.1.0 milestone Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants