Skip to content

Gtk UI: Handle links in HTML shownotes #1280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
elelay merged 1 commit into from
Apr 24, 2022
Merged

Gtk UI: Handle links in HTML shownotes #1280

elelay merged 1 commit into from
Apr 24, 2022

Conversation

@thp
Copy link
Member

@thp thp commented Apr 22, 2022

Noticed that in HTML shownotes, clicking links (with left mouse button) does nothing. Hook up the handler to open the link in the default web browser. Add a UI confirmation dialog when clicking links in shownotes (both text and HTML) before opening the browser. This can be disabled by setting the advanced config option ui.gtk.shownotes_confirm_open_browser to False, in which case clicking the link will immediately open the browser.

@tpikonen
Copy link
Contributor

tpikonen commented Apr 22, 2022

I'm not sure if the confirmation dialog is needed. The text shownotes have click-to-open already and the behavior should be the same in both widgets.

@thp
Copy link
Member Author

thp commented Apr 22, 2022

Any preferences @elelay @auouymous? I'm happy to remove the confirmation dialog code if it doesn't add anything of value :)

@thp thp force-pushed the thp/clickable-shownotes-with-confirmation branch from 80c2f8b to 6a0cac4 Compare April 23, 2022 08:21
@thp thp changed the title Gtk UI: Handle links in HTML shownotes, opt-out confirmation dialog Gtk UI: Handle links in HTML shownotes Apr 23, 2022
@thp
Copy link
Member Author

thp commented Apr 23, 2022

Gave this another thought and removed the confirmation UI, since as Teemu said, text-only shownotes behave like this.

So the PR is much simpler now. I still kept the on_link_clicked() function the base class, as it might make things easier in the future if we want to hook something there (implementation will then be shared by both Text and HTML shownotes), even if right now it's just another layer of indirection.

tpikonen
tpikonen reviewed Apr 23, 2022
View reviewed changes
@thp thp force-pushed the thp/clickable-shownotes-with-confirmation branch from 6a0cac4 to 18efca1 Compare April 23, 2022 10:53
@auouymous
Copy link
Member

auouymous commented Apr 23, 2022

LGTM, but I am unable to test it. My only question is why it logs a "refusing to go to ..." message before opening the link?

@thp
Copy link
Member Author

thp commented Apr 23, 2022

LGTM, but I am unable to test it. My only question is why it logs a "refusing to go to ..." message before opening the link?

Historic reasons. We do decision.ignore() so that the web view doesn't navigate to the web page itself. I guess the log message is a bit misleading now, I can remove it.

@thp thp force-pushed the thp/clickable-shownotes-with-confirmation branch from 18efca1 to 3cbb361 Compare April 23, 2022 11:54
@thp
Copy link
Member Author

thp commented Apr 23, 2022

Updated with comment.

@elelay elelay merged commit d21a29b into master Apr 24, 2022
@elelay
Copy link
Member

elelay commented Apr 24, 2022

Tested and merged, thanks!

@elelay elelay deleted the thp/clickable-shownotes-with-confirmation branch April 24, 2022 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tpikonen tpikonen tpikonen left review comments

@auouymous auouymous auouymous left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@thp @tpikonen @auouymous @elelay