Support clicking directly on a URL to open it #2146

trygveaa · 2021-06-23T20:01:09Z

This allows you to click/press directly on a URL in the terminal view to
open it. It takes priority over opening the keyboard, so if you click on
a URL it is opened, and if you click anywhere else the keyboard opens
like before.

Currently, if the application in the terminal is tracking the mouse and
you click on a URL, both actions happen. The mouse event is sent to the
application, and the URL is also opened.

agnostic-apollo · 2021-06-26T19:13:14Z

Ah, finally completed it, eh! Congratz

Sorry for the delay, didn't get time to test. It seems to be working fine on my device without a mouse.
The issue that I see is that if the url doesn't fully cover the whole line and you click on the blank space on the right, it will still open the url. Is that solvable in some way or would be troublesome due to 5f71e3e?

Also should add a key in termux.properties to enable/disable this behaviour, since everybody may not want this and slower devices may be affected too. A lot of things have been added to termux recently and I wouldn't want to slow down the overall user experience, which may not be detectable on relatively faster test devices. Will have to decide whether it should be enabled by default or not though.

Maybe this should also only be triggered if ctrl key is down, like xfce4-terminal doesn't open urls on click, but requires a right click that shows the Open Link option once you hover over a url and it is highlighted and it doesn't have the blank space problem above. Would this also be helpful for mouse issues? What do you think?

Also regex Pattern should be compiled once and not on every touch. I will/can do this and add termux.properties key after the merge since otherwise there will be conflicts with my local master branch if you do it here. Already TermuxTerminalViewClient likely has them and I need to change it and TerminalView further to fix other stuff, so should merge before doing that.

Also it would be better if you added some comments in getWordAtLocation() to explain what that logic is doing so it's easier for the reader to understand and maintain.

Thanks

trygveaa · 2021-06-28T21:21:51Z

Sorry for the delay, didn't get time to test.

No worries, three days isn't that long :)

It seems to be working fine on my device without a mouse.
The issue that I see is that if the url doesn't fully cover the whole line and you click on the blank space on the right, it will still open the url. Is that solvable in some way or would be troublesome due to 5f71e3e?

Ah, I see what you mean, hadn't noticed that. I don't think the commit you're linking to matters for this. I'll take a look at it.

Also should add a key in termux.properties to enable/disable this behaviour, since everybody may not want this and slower devices may be affected too. A lot of things have been added to termux recently and I wouldn't want to slow down the overall user experience, which may not be detectable on relatively faster test devices. Will have to decide whether it should be enabled by default or not though.

Hm, not sure I see how it would be slowed down? It only runs if you tap the screen, and you don't typically do that very often for other things? But sure, we can have an option. I see below that you want to do this?

Maybe this should also only be triggered if ctrl key is down, like xfce4-terminal doesn't open urls on click, but requires a right click that shows the Open Link option once you hover over a url and it is highlighted and it doesn't have the blank space problem above. Would this also be helpful for mouse issues? What do you think?

I would hate that, since I want to be able to open URLs in the easiest way possible (I use IRC and Slack in Termux, so I open URLs all the time). But we could have an option for it.

Also regex Pattern should be compiled once and not on every touch. I will/can do this and add termux.properties key after the merge since otherwise there will be conflicts with my local master branch if you do it here. Already TermuxTerminalViewClient likely has them and I need to change it and TerminalView further to fix other stuff, so should merge before doing that.

Right, I just reused extractUrls without looking much at it.

Also it would be better if you added some comments in getWordAtLocation() to explain what that logic is doing so it's easier for the reader to understand and maintain.

Sure.

I'm on vacation now though, so will do this next week.

agnostic-apollo · 2021-06-28T21:31:03Z

Ah, I see what you mean, hadn't noticed that. I don't think the commit you're linking to matters for this. I'll take a look at it.

Thanks. If it's fixable, then great.

Hm, not sure I see how it would be slowed down? It only runs if you tap the screen, and you don't typically do that very often for other things? But sure, we can have an option. I see below that you want to do this?

I would hate that, since I want to be able to open URLs in the easiest way possible (I use IRC and Slack in Termux, so I open URLs all the time). But we could have an option for it.

Probably, slowing down may not be that much an issue, if regex is precompiled. But would be better to have an option in case someone doesn't want it.

Okay, then we can just leave it at directly opening on touch for now unless some issue arises in future for someone.

Right, I just reused extractUrls without looking much at it.

No worries.

Sure.

Thanks

I'm on vacation now though, so will do this next week.

Yeah, enjoy for now then. Will leave this for next release. I will refactor the extractUrls and add the termux.properties in this release, you can hook into it later.

…n click The user can add `terminal-onclick-url-open` entry to `termux.properties` file to enable opening url links in terminal transcript on click or on tap. The default value is `false`. So adding the entry `terminal-onclick-url-open=true` to `termux.properties` file will enable url opening. Running `termux-reload-settings` command will also update the behaviour instantaneously if changed. This commit just adds the property and doesn't implement the functionality. That will later be merged from #2146.

Needed for #2146.

agnostic-apollo · 2021-06-30T01:26:32Z

You can check 69bebb5 and 28b9f93 for how to hook.

trygveaa · 2021-09-05T12:12:23Z

Sorry for the very long delay here. I've now fixed the issue if you click on the blank space on the right, taken the option you added into use, added some comments and tests for getWordAtLocation and rebased on master.

Btw, one thing I noticed while testing this is that when you long press to select text, an offset is applied so the word on the line above where you pressed is selected. However when mouse tracking is enabled, this offset is not applied, so that means that different lines are affected when you click and when you long press when mouse tracking is enabled.

For URL opening I used the same code to get the position as the long press uses, so that uses the offset.

agnostic-apollo · 2021-09-06T00:10:38Z

No worries about the delay. Thanks for adding the comments and tests. The issues seem to be fixed.

I am not sure why that offset was added, might be because pressing with finger will have a larger touch area and we need to select bottom of line. Is that affecting behaviour for you?

Pull request looks good. I can merge it if you don't plan on changing anything further. I force pushed to use ShareUtils.openURL() instead of keeping duplicated code. The master branch will show the system chooser too as per dd952a9.

trygveaa · 2021-09-06T15:11:06Z

I am not sure why that offset was added, might be because pressing with finger will have a larger touch area and we need to select bottom of line. Is that affecting behaviour for you?

I'm not sure either. Well, it is affecting me in that I have to press a different place when I want to select/open url and when I want to interact with an application with mouse tracking. Not really related to this PR though, other than that I had to choose one of the behaviors when implementing it, and I chose the same as when selecting text because that fit better with the code. But as for the actual behavior I'm not sure I see how the offset is improving things.

Pull request looks good. I can merge it if you don't plan on changing anything further. I force pushed to use ShareUtils.openURL() instead of keeping duplicated code. The master branch will show the system chooser too as per dd952a9.

Great. No, I don't plan any more changes.

agnostic-apollo · 2021-09-06T19:34:11Z

I'm not sure either.

Well, neither the comment, nor the commit message (check top) when it was added in terminal-view has any detail for why. But it was actually added in jackpal source code for "Add a verical offset so you can see what you're selecting.". That was not fun to find, had to jump over multiple file moves!

So it has no significance in actual selection. If its indeed causing problem in selection with accuracy, then maybe it should be removed, unless someone objects.

Great. No, I don't plan any more changes.

If you wanna remove the offset, you can do it here, otherwise will merge.

This allows you to click/press directly on a URL in the terminal view to open it. It takes priority over opening the keyboard, so if you click on a URL it is opened, and if you click anywhere else the keyboard opens like before. Currently, if the application in the terminal is tracking the mouse and you click on a URL, both actions happen. The mouse event is sent to the application, and the URL is also opened. To enable support for this, you have to set `terminal-onclick-url-open=true` in `termux.properties`.

trygveaa · 2021-09-19T16:04:54Z

If you wanna remove the offset, you can do it here, otherwise will merge.

Alright, I've removed the offset and changed the calculation of column number for selection/url to match the calculation used for mouse tracking. See commit 54bb83d for an explanation.

When calculating the row that is clicked, for mouse tracking mFontLineSpacingAndAscent was taken into account, but for selection and URL clicking it wasn't. This adds a common function for calculating the column and row which does take it into account and use that for all three. I'm not quite sure why it's necessary to subtract mFontLineSpacingAndAscent, but with this calculation the click location matches the line that is acted on for me with both touch and mouse and on different font sizes. It also removes the offset for finger the selection/url used because I don't think it's common for apps on Android to have such an offset, and because the mouse tracking did not use such an offset.

agnostic-apollo · 2021-10-13T17:41:59Z

Testing and working fine for both tapping and clicking. Users would need to fix their muscle memory and tap directly on the text since offset has now been removed, but shouldn't be too much of an issue. I think removing offset would also improve selecting text at the bottom of the terminal too.

Thanks for this!

ghost · 2021-10-17T18:21:06Z

will we get this update on playstore or f-droid version of termux app?

ghost · 2021-10-17T18:23:30Z

you can make a popup message which can ask users if they want to leave the app to open the url or not

"do you want to open the url?"
"yes" "no"

agnostic-apollo · 2021-10-17T18:59:26Z

will we get this update on playstore or f-droid version of termux app?

Playstore releases have been deprecated and haven't received an update for over an year.

https://github.com/termux/termux-app#google-play-store-deprecated

you can make a popup message

Can be added later. Also an option to view in browser or webview.

Mark-Joy · 2022-01-16T16:01:04Z

How does this feature work?
I tap on the url: https://google.com
Nothing happened except soft-keyboard is opened.

Click-Url.mp4

agnostic-apollo · 2022-01-16T16:02:47Z

Read changelog.

https://github.com/termux/termux-app/releases/tag/v0.118.0

Mark-Joy · 2022-01-16T16:14:41Z

app version 0.118
terminal-onclick-url-open=true in ~/.termux/termux.properties. termux-reload-settings was complete.
Still nothing when click on url.

Click-url1.mp4

agnostic-apollo · 2022-01-16T16:34:31Z

Exit termux app completely and start again. termux-reload-settings does not work on some devices, granting draw over apps permission may allow it to work.

Mark-Joy · 2022-01-16T16:40:12Z

Turns out that I had a space character after true value in ~/.termux/termux.properties: 😒😒
terminal-onclick-url-open=true[space]

…n click The user can add `terminal-onclick-url-open` entry to `termux.properties` file to enable opening url links in terminal transcript on click or on tap. The default value is `false`. So adding the entry `terminal-onclick-url-open=true` to `termux.properties` file will enable url opening. Running `termux-reload-settings` command will also update the behaviour instantaneously if changed. This commit just adds the property and doesn't implement the functionality. That will later be merged from termux#2146.

Needed for termux#2146.

Added: Allow users to directly open URL links in terminal transcript when clicked or tapped The user can add `terminal-onclick-url-open=true` entry to `termux.properties` file to enable opening of URL links in terminal transcript when clicked or tapped. The default value is `false`. Running `termux-reload-settings` command will also update the behaviour instantaneously if changed. Implemented in termux#2146

…n click The user can add `terminal-onclick-url-open` entry to `termux.properties` file to enable opening url links in terminal transcript on click or on tap. The default value is `false`. So adding the entry `terminal-onclick-url-open=true` to `termux.properties` file will enable url opening. Running `termux-reload-settings` command will also update the behaviour instantaneously if changed. This commit just adds the property and doesn't implement the functionality. That will later be merged from termux#2146.

Needed for termux#2146.

Added: Allow users to directly open URL links in terminal transcript when clicked or tapped The user can add `terminal-onclick-url-open=true` entry to `termux.properties` file to enable opening of URL links in terminal transcript when clicked or tapped. The default value is `false`. Running `termux-reload-settings` command will also update the behaviour instantaneously if changed. Implemented in termux#2146

agnostic-apollo self-requested a review June 26, 2021 19:13

agnostic-apollo added a commit that referenced this pull request Jun 30, 2021

Compile Url match regex once and not on every use

28b9f93

Needed for #2146.

trygveaa force-pushed the click-on-url branch from 1dc0ea4 to e1dbfda Compare September 5, 2021 12:01

agnostic-apollo force-pushed the click-on-url branch from e1dbfda to 16b167b Compare September 6, 2021 00:01

trygveaa force-pushed the click-on-url branch from 16b167b to 25650b5 Compare September 19, 2021 16:02

trygveaa force-pushed the click-on-url branch from 25650b5 to 55af769 Compare September 19, 2021 16:07

trygveaa force-pushed the click-on-url branch from 55af769 to 54bb83d Compare September 19, 2021 16:11

agnostic-apollo merged commit af16e79 into termux:master Oct 13, 2021

trygveaa deleted the click-on-url branch October 13, 2021 17:51

AdamMickiewich pushed a commit to VolyaTeam/dzida-app that referenced this pull request Aug 8, 2022

Compile Url match regex once and not on every use

61c7bc9

Needed for termux#2146.

shrihankp pushed a commit to reisxd/termux-app that referenced this pull request Oct 20, 2022

Compile Url match regex once and not on every use

5a0774b

Needed for termux#2146.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support clicking directly on a URL to open it #2146

Support clicking directly on a URL to open it #2146

Support clicking directly on a URL to open it #2146

Support clicking directly on a URL to open it #2146

Conversation