Hyperlinks with the same ID and different URIs collide when they shouldn't

[PhMajerus]

In Windows Terminal Preview 1.4.2652.0.
Hyperlinks support is a great addition, but from my understanding there is a bug with how links IDs are parsed and handled.

Character cells that have the same target URI and the same nonempty id are always underlined together on mouseover.
The same id is only used for connecting character cells whose URIs is also the same. Character cells pointing to different URIs should never be underlined together when hovering over.
(Source: https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#hover-underlining-and-the-id-parameter )

But testing the same ID with different URIs shows Windows Terminal is grouping them together and using the URI of the last one for both :
echo -e '\e]8;id=a;http://www.microsoft.com/\aMicrosoft\e]8;;\a \e]8;id=a;http://www.google.com/\aGoogle\e]8;;\a'

Even different IDs are grouped together if they are not numbers:
echo -e '\e]8;id=a;http://www.microsoft.com/\aMicrosoft\e]8;;\a \e]8;id=b;http://www.google.com/\aGoogle\e]8;;\a'

While the page about hyperlinks explicitely shows and states that these IDs are strings, not numbers:

params is an optional list of key=value assignments, separated by the : character. Example: id=xyz123:foo=bar:baz=quux.

Complex apps that display data that might itself contain OSC 8 hyperlinks (such as terminal multiplexers, less -R) should do the following: If the encountered OSC 8 hyperlink already has an id, they should prefix it with some static string, or if multiple windows/panes are supported by the app, a prefix that's unique to that window/pane to prevent conflict with other windows/panes.

[DHowett]

So, a (uri 1) a (uri 2) is operating as designed. The reuse of an ID with a new URI is utterly intended to replace the old ID.

I can't reproduce your second issue, a (uri 1) b (uri 2):

Complex apps ...

Since Terminal's "terminals" are individual entities with their own hyperlink stores and user interactivity, we don't fall into the "display data that might itself contain OSC 8 hyperlinks" case and are therefore not in need of namespacing. Two hyperlinks in two different panes are automatically in different isolated roots:

note only the link in the right pane is highlighted

[DHowett]

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

Ah. HM.

[DHowett]

Why would there be an ID parameter if it cannot be used to disambiguate URIs? Why would you reuse the same ID? I've got so many questions.

[PhMajerus]

@DHowett Yeah, the one with different ids a and b and different URIs seems to be processed as separate, not sure what I did when testing, sorry.

Why would there be an ID parameter if it cannot be used to disambiguate URIs? Why would you reuse the same ID? I've got so many questions.

That one I can answer!
Imagine a CUI utility that shows several files and wants to hyperlink them, you don't want to generate lots of IDs that could collide with IDs that were already output by other CUI utilities.
So that utility could generate a single random ID for its complete output, but have different URIs for each file, instead of having to generate as many IDs as it has to show hyperlinks that are guaranteed to have different URIs.
If the ID generation is based on milliseconds, it's better not to generate a lot of them within a single utility.

This is exactly what I'm doing in one of my utility to have hyperlinks spanning several lines of a grid:

They all share a single ID, but URIs are differentiating them.

[DHowett]

interesting! I'd've never expected that that might rely on {id, uri} tuples that differed only on the uri

[PhMajerus]

Yeah, I'm basically using a link ID generator function and try to keep the same ID for a single output.
Calling that ID generator on each line would be uneccessary and pollute the IDs hash table for no good reason.

[DHowett]

I think that's just hiding the pollution somewhere you don't have control over ;P because somewhere the terminal emulator needs to store a list of IDs and how they map to URIs, and unique across different URIs, which I expect inevitably pollutes somebody's ID hash table... right?

My quick fix is...

diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp
index 294813a3f..cac3718f4 100644
--- a/src/buffer/out/textBuffer.cpp
+++ b/src/buffer/out/textBuffer.cpp
@@ -2279,32 +2279,35 @@ std::wstring TextBuffer::GetHyperlinkUriFromId(uint16_t id) const
     else
     {
         // assign _currentHyperlinkId if the custom id does not already exist
-        const auto result = _hyperlinkCustomIdMap.emplace(params, _currentHyperlinkId);
+        std::wstring newId{ id };^M
+        // hash the URL and add it to the custom ID - GH#7698^M
+        newId += L"%" + std::to_wstring(std::hash<std::wstring_view>()(uri));^M
+        const auto result = _hyperlinkCustomIdMap.emplace(newId, _currentHyperlinkId);^M

which is pretty much that ;P

[DHowett]

sidebar

You're probably more impacted by the fact that we reject file URIs, right? We were having a long discussion over on #7526 about it, and never came to anywhere we felt comfortable with. The same discussion is playing out in #7668, because of how hard it is for the Terminal alone to map paths...