HyperTextView

[marbetschar]

This PR adds Granite.Widgets.HyperTextView , a drop in replacement for Gtk.TextView with added support for navigatable URI's. The code is mainly based on the example of Alberto Fanjul:

Although it is not heavily tested yet, it should work for the majority of cases. Also I guess there is room for performance improvements (see discussion in elementary/calendar#656).

This feature request came up in Calendar (elementary/calendar#571), but would also be useful in descriptions for Tasks and potentially further cases.

Last but not least, this PR is a response to @cassidyjames request on Twitter: https://twitter.com/CassidyJames/status/1404848863395991558

[mcclurgm]

I like this idea, although I haven't had the chance to test it yet. This definitely seems like a good thing to implement in Granite imo, since it seems like a pain to write but useful for a wide variety of apps.

I think doing something like pulling up a long blog post's source code on weaker hardware like a Pi or Pinebook would be a good stress test for performance, but I don't have that hardware. I'll try pulling up a long editor sometime today if I have the time, but my computer's pretty fast, so I expect it can handle this pretty well.

A few thoughts in general:

• How generic do we want the URI regex to be? See my comment in Clickable links in event descriptions calendar#656, you can get very in the weeds with trying to detect a generic URL: https://mathiasbynens.be/demo/url-regex
• One random thought about performance: is there a way to restrict the regex search to only a part of the buffer? If so, we could restrict it to only an area that we know has been modified, since that's the only place that a previously undetected link could appear. AFAIK it's impossible to have a line break in a URI, so we could search only in the paragraph that was edited, for example. This would require some special casing for pastes and such that can edit more than 1 paragraph at a time. I'm not familiar with Gtk text widgets to say how we could handle that. But if this is possible to implement cleanly, it should let us parse arbitrarily long text buffers, as long as people don't go crazy with making long paragraphs. There may be other ways to divide up the buffer too, but that's the first that came to mind.
• Like I said I haven't tested this, but does it handle deleting part of a link, to the point where it becomes invalid? That should make the link markup disappear. (This is more a reminder to myself to test.)
• Do we want to make the markup style configurable? IE, specify color and maybe bold/italic/underline? That would definitely increase the scope though.

[marbetschar]

@mcclurgm I do agree with the ideas on how to improve performance, but I'm wondering if the effort is really worth it. So having someone doing some real world testing on restricted hardware would be great.

Regarding the link detection; The regex in place right now is rather simple. Leading and trailing whitespace mark the start and end of the URI. In between there must be at least four non-whitespace characters with a dot in the middle (at least 2 characters, 1 dot, at least 2 characters). The protocol information at the beginning (xyz://) is optional and HyperTextView will use http:// if there isn't any protocol.

I'm aware this kind of regex will lead to some false positives (ironically "Gtk.TextView" is a false positive) - but its simple and I can't think of anything it does not work for. IMHO this outweights the false positives - at least from what I've found so far ;)

UPDATE: While the basic idea of the regex is still the same (protocol optional, two chars, a dot, two chars), I expanded it a bit to add support for some more common use cases. See comment below.

[marbetschar]

Here's the result of running the examples against the regex currently in place: https://regex101.com/r/bmX4TC/1

UPDATE: Thinking about the current regex a bit more, it seems like file paths or names are probably common false positives (something like /home/user/Downloads/Test.zip, ~/Downloads/Test.zip or simply Test.zip).

Wondering if its enough if we should just use file:// instead of http:// in case the string starts with either / or ~?

Also parentheses are a thing: "(Test.zip)" is false positive too. Wondering if there's an easy way to strip unwanted chars which is not too restrictive?

UPDATE:

• Added support for local file paths (needs to start either with / or ~)
• do not allow quotation marks or < or > to be part of the URI (improves HTML/XML support)
• Ignore strings which start or end with parentheses for improved Markdown support ([, ( respectively ), ])
• https://regex101.com/r/VFMdIS/1 shows the new behaviour

[marbetschar]

@mcclurgm adressed the performance concerns: Rescanning of the buffer is now limited from line start to line end of the cursor position. Also buffer change events are debounced. That said, scanning of said substring happens once each 300ms at most. The only exception is when text was pasted - in this case a rescan of the whole buffer is forced after 300ms.

This strategy seems to work quite well on my end - even for large texts. As test case I copied the source of https://blog.elementary.io/elementary-os-6-odin-beta-2/ (around 1300 lines of code) and editing feels normal on my machine.

[mcclurgm]

I'm seeing some strange behaviors here. First, it seems like there's some issue with offset. Depending on what I paste, the hypertext underlines start after the URI does, and seem to extend past the end. Here's an example:

/*
 *  Copyright (C) 2019–2020 elementary, Inc. (https://elementary.io)
 *                2011–2013 Maxwell Barvian <[email protected]>
 *
 *  This program or library is free software; you can redistribute it
 *  and/or modify it under the terms of the GNU Lesser General Public
 *  License as published by the Free Software Foundation; either
 *  version 3 of the License, or (at your option) any later version.

On my machine, I see that the link underlines start after the ht and maxw text. Depending what I select, the offset varies. For example, just pasting

Copyright (C) 2019–2020 elementary, Inc. (https://elementary.io)
 *                2011–2013 Maxwell Barvian <[email protected]>

the link underlines start after https: and maxwell@. They also extend past the end of the URI: in the second example, the elementary.io link extends a few characters into the next line (it ends a few spaces after the *). As a counterexample, this text behaves perfectly normally when pasted:

Someone is inviting you to a scheduled Zoom meeting.

Join Zoom Meeting
https://abcd.zoom.us/j/0123456

Meeting ID: 904 938 668

One tap mobile
+10302023418# US (San Jose)
+12343210,,# US (San Jose)
(https://elementary.io)

Dial by your location
        +1 832 120 9417 US (San Jose)
Find your local number: https://abcd.zoom.us/u/jfdklsajfds

I see some strange behavior with links surrounding the insertion point blinking in and out when editing by hand, when I was looking at the offset issue. I've attached a video. Sorry about the artifacts. Focus on the elementary.io links on either side of the cursor.
https://user-images.githubusercontent.com/29022469/123734185-1b344a00-d85a-11eb-9a35-53470518ce96.mp4

Then another (maybe) unrelated bug. The regex (sometimes?) doesn't match at the very end of a buffer. For example, if I paste the second example string (the shortened copyright header) at the end of the file, neither URI matches. If I add a line break after that same text, the link will sometimes match. But sometimes I paste a long buffer with a link at the end and it works ok? This is a weird one too. I haven't found any indication of when it works or doesn't.

Also, some things that aren't bugs but I wanted to bring up:

• When you type at the end of a link, it extends as you type. This looks a little odd until the debouncing catches up and the underline disappears. Maybe we should keep the link constant until the next parse? I can see either way on this.
• The link is followed on mouse down, but it should probably be mouse up to follow how other buttons work in the system. (For example, I can't select any text in a link with the mouse because the link just opens.)
  • Should we do something like ctrl+click to activate, at least as an option that the client can set? Some editors do that.

I can try to look into the cause of the bugs I mentioned, but I'm not really sure when I'll have time, sorry.

[marbetschar]

@mcclurgm good catch - that was a tricky one! The wrong behaviour was because of multibyte string encoding. Or as the documentation puts it:

Character counts are usually referred to as offsets, while byte counts are called indexes. If you confuse these two ... bad things will happen - Text Widget Conceptual Overview

I fixed the error and detection works fine now even for more complex multi-byte-strings with emojis or similar. Feel free to re-test.

Regarding the Ctrl + Click: I like the idea, but unfortunately I had no luck showing a tooltip indicating this behaviour. Maybe someone else can take over here?

[mcclurgm]

Aha--good catch. The joys of text rendering! 😬

In some very quick testing I haven't found any issues with the end of the buffer, so it's possible that was related to the offsets too. I'll let you know if I see it again.

Interesting point about the tooltips. I found a mailing list thread with some cursory duckduckgo, but it looks complicated, and the thread never comes up with a complete solution. So you're right, that may be too hard to pull off. As a potential compromise, what if we only followed the link if the mouse releases close to the position where it was pressed? That would allow you to select text without following the link. (For prior art, this is how Mac's TextEdit does it. I know Google Docs shows a popup with a clickable link when you click on link text, and I think Word uses ctrl+click.) I'm not sure how easy a "click" vs a "selection drag" would be to define though: we'd probably have to make some thresholds.

[marbetschar]

I'm up for anything which improves the experience. Maybe someone from @elementary/ux has got some additional thoughts on this?

[mcclurgm]

I'm checking out the Tasks PR to see. From my perspective from what I've seen in Calendar, this seems good to me though.

I don't understand what you mean by the popover in Tasks. Is that a part of HyperTextView?

One question though: is it possible to have links without a domain name? I get that something like https://e is highlighted as a link. That may be something to fix if that's an invalid hyperlink address. I'll post a suggestion that would fix this is you want.

[marbetschar]

@mcclurgm thanks for your feedback!

is it possible to have links without a domain name? I get that something like https://e is highlighted as a link. That may be something to fix if that's an invalid hyperlink address. I'll post a suggestion that would fix this is you want.

I think we can safely assume there needs to be at least one . in an url for now. I also like the idea of getting rid of the string concatenation, so I adjusted the regex accordingly!

However, I wasn't sure what the ? before the dot is supposed to do - so I omitted it and the regex now reads: @"https?:\\/\\/$(http_charset)+\\.$(http_charset)+" - which does work fine on my end.

I don't understand what you mean by the popover in Tasks. Is that a part of HyperTextView?

Yes, the popover is part of HyperTextView and appears if you hover over a hyperlink and wait for a bit. It should also appear in Tasks, but does not right now - no clue why. But I think that's a minor issue and related to the Tasks app, so we should be good to merge HyperTextView regardless.

Here's how it looks like in the granite-demo app:

[mcclurgm]

However, I wasn't sure what the ? before the dot is supposed to do - so I omitted it and the regex now reads: @"https?:\/\/$(http_charset)+\.$(http_charset)+" - which does work fine on my end.

That should work just as well, I think. The ? is a "non-greedy quantifier," so it matches as few instances of the pattern as possible. So if you have multiple dots, it would capture only up to the first one with the first quantifier, then the rest would be captured by the next quantifier. In our case, it should be indistinguishable except for subpatterns, which we aren't using. It also doesn't behave quite how I initially though with regard to subpatterns, so let's keep it how you implemented it.

I am seeing the tooltip in Tasks, but strangely it's intermittent and only appears on certain links some of the time. I may try to track it down, but if you think it's Tasks-related, that doesn't have to be a blocker for this PR.

[marbetschar]

Thanks to the engineering work of @mcclurgm and @jeremypw we even fixed the tooltip issue in the Tasks app now (elementary/tasks#281) - thank you both for this missing piece!

We are now at a point with HyperTextView where I'm fully confident that HyperTextView provides a smooth experience and I'm not aware of any issues. So IMHO we are here finally able to merge - thanks for this team effort! 🥳

[mcclurgm]

For what it’s worth I’d say LGTM. Makes sense to get a final review from Danielle though.

I have some thoughts for enhancement to discuss, but let’s wait on those until we commit a functional version. (Mostly just colors and branding could be nice to customize, and also add documentation. I'd be happy to write documentation myself, once the final form is figured out.)

Requested changes were made

[danirabbit]

Let's :)