After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776903 - Label with hyperlinks cannot be opened with touch on wayland
Label with hyperlinks cannot be opened with touch on wayland
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkLabel
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 785339 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-01-05 14:42 UTC by Jan-Michael Brummer
Modified: 2017-07-26 14:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch fixing link handling with a tablet under wayland (1.01 KB, patch)
2017-03-26 19:20 UTC, Jan-Michael Brummer
needs-work Details | Review
gtklabel: Fix touch link handling under wayland (3.06 KB, patch)
2017-07-25 15:24 UTC, Carlos Garnacho
committed Details | Review

Description Jan-Michael Brummer 2017-01-05 14:42:24 UTC
Labels with hyperlinks attached (e.g. about dialog url/emails) cannot be visited using touch input under wayland (X works fine).

Reproduce: gtk3-demo -> Links, or about dialog in every GNOME application.
Comment 1 Jan-Michael Brummer 2017-03-26 12:59:19 UTC
Issue is caused by missing GDK_MOTION_NOTIFY under wayland for the tablet input. This leads to an empty active_link in GtkLabel as gtk_label_motion() is never called.

One fix (that works) is calling gtk_label_motion() in gtk_label_multipress_gesture_pressed() before checking active_link.
Comment 2 Jan-Michael Brummer 2017-03-26 19:20:18 UTC
Created attachment 348745 [details] [review]
Patch fixing link handling with a tablet under wayland
Comment 3 Jan-Michael Brummer 2017-03-30 19:52:04 UTC
Anybody ready for a review?
Comment 4 Carlos Garnacho 2017-07-25 15:23:01 UTC
Comment on attachment 348745 [details] [review]
Patch fixing link handling with a tablet under wayland

Thanks for the patch. The analysis is right, I however don't think it is correct to call the label motion handler manually. Most importantly it's wrong to chain up on the motion-notify vmethod here as it would be the case, it's also wrong to pass other event than GDK_MOTION_NOTIFY to such vmethod (you'll receive GDK_BUTTON_PRESS or GDK_TOUCH_BEGIN here, depending on the widget event mask). It happens to work by coincidence because the event structs are binary compatible for the fields that are looked up.

But the fix is not totally off, we just need refactoring the "update active link" bits into a separate event-agnostic function and call it from both the widget motion and the gesture press handlers. I'm attaching here a new patch doing that.
Comment 5 Carlos Garnacho 2017-07-25 15:24:07 UTC
Created attachment 356366 [details] [review]
gtklabel: Fix touch link handling under wayland

Refactor the code updating the active link under the current coordinates
into a separate function, and call it on GtkGestureMultiPress::pressed
so the link is updated on GDK_TOUCH_BEGIN. Based on a patch by
Jan-Michael Brummer <jan.brummer@tabos.org>.
Comment 6 Jan-Michael Brummer 2017-07-25 15:48:01 UTC
Thank you very much for the review. Your patch looks much better and it works well on my tablet.
Comment 7 Jan-Michael Brummer 2017-07-26 09:22:46 UTC
Carlos, does your patch needs another reviewer or can it be applied to git?
Comment 8 Carlos Garnacho 2017-07-26 11:05:55 UTC
Thanks, it's gotten enough testing :). Pushed to git.

Attachment 356366 [details] pushed as d6d4217 - gtklabel: Fix touch link handling under wayland
Comment 9 Carlos Garnacho 2017-07-26 14:20:04 UTC
*** Bug 785339 has been marked as a duplicate of this bug. ***