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

This addon breaks xterm.js fonts #839

Open
ngaro opened this issue Sep 11, 2021 · 10 comments
Open

This addon breaks xterm.js fonts #839

ngaro opened this issue Sep 11, 2021 · 10 comments

Comments

@ngaro
Copy link

ngaro commented Sep 11, 2021

Description

All sites that use xterm.js show a "broken" version of the fonts.

Steps to Reproduce

  1. Surf to https://xtermjs.org/ to see the broken fonts
  2. Disable this addon
  3. See how the fonts should look

Versions

  • Extension: 2021.9.2
  • Browser: Firefox 92.0
  • OS: Mint 20.2

Once the fonts look fine again, you can usually enable the addon again without reintroducing the problem.

@kdzwinel
Copy link
Member

kdzwinel commented Sep 17, 2021

Thanks for reporting! I followed the steps that you outlined and I wasn't able to reproduce (same extension version, same FF version, but on MacOS) 🤔 This may have something to do with our canvas protection, but I'll need more reliable repro case to investigate. Also, can you please share the screanshot of broken fonts?

@jonathanKingston can you please double-check on Linux?

@ngaro
Copy link
Author

ngaro commented Sep 17, 2021

Results on the xterm.js site with addon enabled:
privacyaddonenabled and disabled:
privacyaddondisabled

@ngaro
Copy link
Author

ngaro commented Sep 17, 2021

PS: If you don't have a Mint 20.2 nearby, it is based on Ubuntu 20.4.
I assume it will have the same problem

@jonathanKingston
Copy link
Collaborator

I have Ubuntu 20.4 with various Firefox installs and can't reproduce.

I can't see the code doing any font sizing via Canvas, which leads me to think this is either:

  • A different set of system fonts
  • Firefox doing some weird rendering that might be fixed by a reload?

Is the problem still reproducible for you @ngaro?

@ngaro
Copy link
Author

ngaro commented Sep 28, 2021

Yes, now I also reproduced it in a VM at the obfuscated link below:

Start with "https" follow it by a colon and 2 slashes, follow it by "login-cmt.uantwerpen.COUNTRYCODEOFBELGIUM/vmmintddg831.ova" and replace COUNTRYCODEOFBELGIUM by the correct one. It's a large download (almost 3GB) so won't keep it online forever.

@ngaro
Copy link
Author

ngaro commented Sep 29, 2021

Sorry, I just noticed that i gave the file wrong permissions, download should work now

@shodanx2
Copy link

shodanx2 commented Oct 3, 2021

Hello,
I would like to point out this issue also affects the Home Assistant SSH terminal web ui add on

See github issue home-assistant/addons#2214

This probably breaks other things too, only current solution is to disable the duck duck go plugin

Note, I don't have this problem on https://xtermjs.org/

I have right now the plugin enabled

This is https://xtermjs.org/
image

And here is my home assistant
image

@GGORG0
Copy link

GGORG0 commented Oct 17, 2021

Hello, I would like to point out this issue also affects the Home Assistant SSH terminal web ui add on

See github issue home-assistant/addons#2214

This probably breaks other things too, only current solution is to disable the duck duck go plugin

Note, I don't have this problem on xtermjs.org

I have right now the plugin enabled

This is xtermjs.org image

And here is my home assistant image

same problem here, but with ttyd instead of home assistant (home assistant uses ttyd). same goes for xterm.js - it works fine there

Progams:

  • VSCode web: problems
  • ttyd: problems
  • portainer: works fine
  • jupyterlab: works fine
  • coderpad: works fine
  • xtermjs.org: works fine

after whitelisting the sites in the add-on settings, they work fine

@jonathanKingston
Copy link
Collaborator

I suspect this is the fingerprint randomisation with their glyph sizing code: https://github.com/xtermjs/xterm.js/blob/569708677fab0b46689bb6cbff9ce7dfce20c38d/src/browser/renderer/CustomGlyphs.ts

It's likely it isn't consistent with which domains we cause this issue to as the browser uses different keys per user to generate this.

We have some carve outs on these protections however it looks like they're drawing bezier curves and other things that would trigger our protections.

One area we could exclude is the check for transparency here: https://github.com/xtermjs/xterm.js/blob/376b29673ba174934b1b6339ef3eed8449fec529/src/browser/ColorManager.ts#L167-L193

However we could be breaking https://github.com/xtermjs/xterm.js/blob/7bd7967ab7c61e544c597e6e1b4fdfc2ee7f73e0/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts#L408-L413 also and I can't see how we'd exclude this.

@shodanx2
Copy link

As a temporary fix, could it be possible to set an exception for DDGPE to not run at all on the following ?

http://homeassistant.lan
https://homeassistant.lan
http://homeassistant.lan:8123
https://homeassistant.lan:8123

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

No branches or pull requests

5 participants