contour-terminal / contour

Modern C++ Terminal Emulator
http://contour-terminal.org/
Apache License 2.0
2.3k stars 100 forks source link

OSC-8 link storage(?) broken for links generated with aerc colorize #1499

Closed ferdinandyb closed 1 week ago

ferdinandyb commented 2 months ago

Contour Terminal version

Contour Terminal Emulator 0.4.4-master-521b1408

Installer source

Github: source code cloned

Operating System

Ubuntu 22.04

Architecture

x86-64

Other Software

Colorize filter from aerc filters. Probably shouldn't be hard to build, but if you tell me what logs to make I can paste that as well.

Steps to reproduce

I was running the below script:

#!/bin/bash

for (( i = 0; i < 20; i++ )); do
  echo "http://example$i.com" | ~/.local/libexec/aerc/filters/colorize -8
  # printf '\033]8;;http://example%s.com\033\\This is a link %s\033]8;;\033\\\n' "$i" "$i"
done

but actually two different links are enough to reproduce. Contour will consistently open the link corresponding to first link that was output by colorize in the terminal. A subsequent osc-8 with the commented out printf will work fine, but any osc-8 output by colorize will forever open the same (first) link.

Expected Behavior

The proper links are opened.

Actual Behavior

Contour will consistently open the link corresponding to first link that was output by colorize in the terminal. A subsequent osc-8 with the commented out printf will work fine, but any osc-8 output by colorize will forever open the same (first) link.

Additional notes

No response

Yaraslaut commented 1 month ago

We see this bug because colorize.c sets id for the url, some info about id you can find here: https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda While initial intention is to use id only for Hover underlining we also use it to open links. When user opens a link, we search for it by id inside internal url cache, and since id is the same for all of them only first link is exist in the cache.

Possible solution is to ignore user specified id since we set it anyway, and when we count it internally then everything will go smooth
Maybe @christianparpart has a better idea

j4james commented 1 month ago

@Yaraslaut Note this line from the spec:

The same id is only used for connecting character cells whose URIs is also the same.

So if you used a combination of the url and the id for the cache key, then you shouldn't have this problem.

Yaraslaut commented 1 month ago

@Yaraslaut Note this line from the spec:

The same id is only used for connecting character cells whose URIs is also the same.

So if you used a combination of the url and the id for the cache key, then you shouldn't have this problem.

You are right, essentially this is equivalent to using our own id, instead of user provided one