re-eval previous active link

[jerch]

Fixes #4295.

Can be tested with this shell command:

echo -n "http://example.com/" && sleep 1 && echo -n "a" && sleep 1 && echo -n "b" && sleep 1 && echo -n "c" && sleep 1 && echo -n " x" && sleep 1 && echo -n "x" && sleep 1 && echo "x" && sleep 1 && echo "y" && sleep 1 && echo "y" && sleep 1 && echo "y"

[Tyriar]

This fixes the case where the link remains on the line, but not when the buffer shifts:

Do you think that's doable? If not or it's hard we can still merge this and keep the bug open.

[jerch]

Do you think that's doable?

I think thats doable yes, but isnt this a contradiction to general interaction pattern? I mean - the viewport moved the link away from current mouse position - who would expect it to stay active? Maybe I do not fully understand the issue yet...

[Tyriar]

@jerch I think ctrl+click should activate whatever is below it, regardless of whether the mouse has moved or not.

I just tried an experiment in Edge by alt tabbing away and back while ctrl is held and it does not show the underline (makes sense as the ctrl event didn't happen on the page), but it does activate when you click. How we currently behave though is we neither show the underline or activate on click, since the underline is required to activate the link for us.

[jerch]

I think ctrl+click should activate whatever is below it, regardless of whether the mouse has moved or not.

You mean like it should hover viewport content, that got moved by viewport changes under a resting pointer? I am not quite sure if thats possible, as we have no valid mouse event in that case - for the browser nothing really changed and the mouse did not move at all, thus no event to chew on. I think thats what you see atm - it still needs that tiny little mouse move to get an event to work with (and then the hovering happens).

What might be possible - grab ALL mouse events at a higher level within the xterm.js container, and store that. On scroll buffer changes that event could be used to derive the pointer position and feed that into the linkifier. But I am not sure, if that "grab all mouse events" idea is so great, esp. perfwise.

[Tyriar]

Actually I'm not sure why the change in this PR doesn't do what I'm describing as it essentially just needs on rendered viewport change -> e-eval.

Also I was saying ctrl, that's just me confusing vscode's one that requires ctrl to be held down.

[Tyriar]

Oh I see now, it's only re-evaluating when a link exists

[Tyriar]

Adding this to attachToDom gets the behavior I'm describing:

    this._renderService.onRenderedViewportChange(e => {
      if (this._lastMouseEvent) {
        this._handleMouseMove(this._lastMouseEvent);
      }
    });

I think we could tweak that to not run it every time, say only when e is the whole viewport, indicating a scroll (or onScroll?), not just a single line? Then add some debouncing in the case of a lot of scroll events.

[jerch]

Yeah sounds doable this way. Still I wonder if we manage to get that probably long-lasting event properly invalidated. Main issue I see here - ppl often park the pointer close to the active spot they are working in, thus we have a high chance to have a parked mouse on top of the terminal widget (or multiple tabbed terminal widgets), where the linkifier would cause a lot of nonsense work from buffer scrolling, hmm.
Also alt-tabbing + in between mouse moves will not get spotted, thus might show wrong underlines when tabbing back.

I really wish there was a way to actively ask the DOM for current mouse position.

[Tyriar]

ppl often park the pointer close to the active spot they are working in, thus we have a high chance to have a parked mouse on top of the terminal widget (or multiple tabbed terminal widgets), where the linkifier would cause a lot of nonsense work from buffer scrolling, hmm.

At least for most cases the link detection would be gated by ctrl/cmd being held down.

Also alt-tabbing + in between mouse moves will not get spotted, thus might show wrong underlines when tabbing back.

I really wish there was a way to actively ask the DOM for current mouse position.

Yeah, only so much we can do. Listening to the mouse outside the page is weird in the browser

[Tyriar]

This beats current, I'll create a new issue to capture the case I mentioned

[Tyriar]

#4323