xtermjs / xterm.js

A terminal for the web
https://xtermjs.org/
MIT License
17.33k stars 1.61k forks source link

Not sure second link in monitor not linked #4964

Closed ldijkman closed 5 months ago

ldijkman commented 6 months ago

hello

Not sure second link in monitor not linked ad on weblinks?

playing with your demo https://ldijkman.github.io/async-esp-fs-webserver/WebSerialMonitor.html

serial monitor demo does not make a second link clickable i think

Tuesday, February 20 2024 17:06:33
mDNS at http://garage.local
Browsing for service _http._tcp.local. ... 1 service(s) found
  1: http://kitchen.local - http://10.10.100.100:80

http://garage.local/ and http://kitchen.local/ is a clickable link

http://10.10.100.100:80 is not a clickable link

Screenshot from 2024-02-20 18-02-53

jerch commented 6 months ago

~Prolly the same as #4296. (I'll keep this issue open until it is confirmed...)~

Nope its not related...

Edit: Cant repro it on master - can you retry with the newer @xterm packages (beta only atm)?

ldijkman commented 6 months ago

hmm thought it was the second link on line is not clickable so changed it but if the ip is the first link it is also NOT clickable

Thursday, February 22 2024 04:08:13
mDNS at http://garage.local
Browsing for service _http._tcp.local. ... 1 service(s) found
  1: http://kitchen.local - http://10.10.100.100:80
  1:  http://10.10.100.100:80

Screenshot from 2024-02-22 05-08-40

ldijkman commented 6 months ago

looks like :port in link is the problem

last ip without port is clickable

Thursday, February 22 2024 04:19:04
mDNS at http://garage.local
Browsing for service _http._tcp.local. ... 1 service(s) found
  1: http://kitchen.local - http://10.10.100.100:80
  1:  http://10.10.100.100:80
  1:  http://10.10.100.100

Screenshot from 2024-02-22 05-19-30

ldijkman commented 6 months ago

second link on monitor line ip without port is clickable

links with port are not clickable links

Screenshot from 2024-02-22 05-35-57

ldijkman commented 6 months ago

hmm

looks like it also does not make links of text with caps

http://Living.local

is not linked

maybe because of L

jerch commented 6 months ago

This roughly summarizes, whats going here:

> new URL('hTTpS://Living.local:443/inPathUpperNotLowered')
URL {
  href: 'https://living.local/inPathUpperNotLowered',
  origin: 'https://living.local',
  protocol: 'https:',
  username: '',
  password: '',
  host: 'living.local',
  hostname: 'living.local',
  port: '',
  pathname: '/inPathUpperNotLowered',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

We currently do a sanity check against the URL parsing result with s.startswith, here we lose the uppercase matching. We should instead do a s.toLowerCase().startswith check. For uppercase in the protocol we can change the regexp to

const strictUrlRegex = /(https?|HTTPS?):[/]{2}[^\s"'!*(){}|\\\^<>`]*[^\s"':,.!?{}|\\\^~\[\]`()<>]/;

instead (assuming those funny mixed case things like 'hTTp' is not what we want to match).

I still cannot repro the port issue, startswith should still match even with the port notion being removed. Have you tested with the newer @xterm packages?

ldijkman commented 6 months ago

Sorry i do not have the knowledge how to change the webadonlinks xterm

I use the chromium terminal example https://ldijkman.github.io/async-esp-fs-webserver/WebSerialMonitor.html

https://github.com/GoogleChromeLabs/serial-terminal

They at chromium said xterm webaddonlinks maybe the problem for not linking

And the ESP mdns monitor links are http not https as your test showed

Greet luberth

jerch commented 6 months ago

@ldijkman PR #4965 should fix the issues. Thx for alot for finding them.