Closed Bug 1568722 Opened 6 years ago Closed 3 months ago

Kinetic scrolling on Linux ought to stop on two-finger touch (hold gestures)

Categories

(Core :: Widget: Gtk, enhancement, P3)

Product:
Core
Component:
Widget: Gtk
Version:
69 Branch
Platform:
Unspecified
Linux
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
133 Branch
Tracking Flags:
Tracking Status
relnote-firefox --- 134+
firefox133 --- disabled
firefox134 --- fixed

People

(Reporter: ryan.hendrickson, Assigned: botond)

References

Details

Attachments

(5 files, 1 obsolete file)

[WIP] Backport the GDK layer of touchpad hold gesture support to GTK 3
13.36 KB, patch
Details | Diff | Splinter Review
[WIP] Hold gesture support in Firefox (incomplete)
5.81 KB, patch
Details | Diff | Splinter Review
gtk3-touchpad-hold.patch
25.54 KB, patch
Details | Diff | Splinter Review
wip1.patch
57.20 KB, patch
Details | Diff | Splinter Review
Bug 1568722 - Handle Linux hold gestures in APZ. r=mstange,dlrobertson
48 bytes, text/x-phabricator-request
Details | Review

Spinning this bug off from my comment on bug 1213601:

[O]nce a two-finger fling has begun, I expected that touching the trackpad again with two fingers would stop the fling immediately, like on macOS (I see from bug 1554456 that this briefly regressed, but that looks fixed now). Instead, it seems that I have to actually scroll a little bit to stop it.

It appears that this would be blocked on a new libinput event type being defined, but if and when libinput makes that change, and GTK adds it to their API, Firefox should support it.

OS: Unspecified → Linux
Priority: -- → P3

Bugbug thinks this bug is a enhancement, but please change it back in case of error.

Type: defect → enhancement

I double-checked that Firefox doesn't currently receive any events when the fingers touch down in such a case, suggesting that we do indeed need to wait for the mentioned libinput / GTK enhancements to make this work.

Depends on: 1213601
Status: UNCONFIRMED → NEW
Ever confirmed: true

Hello,

I just wanted to mention, Chrome's Wayland implementation somehow offers this functionality already. Could someone familiar with Chromium see how they're doing this so that FF might implement this feature in a similar fashion?

(In reply to Ari from comment #3)

I just wanted to mention, Chrome's Wayland implementation somehow offers this functionality already.

Could you provide some details on your Chrome version, how you're running it (to get it into Wayland mode), and your desktop environment please?

I tested this with Chrome 89, run with the --enable-features=UseOzonePlatform --ozone-platform=wayland command line flags to get it into Wayland mode, on Debian 10 in a "GNOME (Wayland)" session, and touching my fingers down did not stop kinetic scrolls.

(In reply to Botond Ballo [:botond] from comment #4)

(In reply to Ari from comment #3)

I just wanted to mention, Chrome's Wayland implementation somehow offers this functionality already.

Could you provide some details on your Chrome version, how you're running it (to get it into Wayland mode), and your desktop environment please?

I tested this with Chrome 89, run with the --enable-features=UseOzonePlatform --ozone-platform=wayland command line flags to get it into Wayland mode, on Debian 10 in a "GNOME (Wayland)" session, and touching my fingers down did not stop kinetic scrolls.

I just realized one thing that I should have really mentioned. It didn't cross my mind until just now. If you look at the mentioned libinput issue, one of the developers was nice enough to give me a patch that introduces the feature for libinput. (Although it is important to mention he has stated the patch is "hacky" and is not appropriate for widespread use.)

The Wayland version of Google Chrome (using the flags you mentioned), can somehow handle the feature from this patch while Firefox cannot. So, I don't think it maters as much any more given the patch is intended as a quick fix.

I'm still sort of curious how Chrome is doing this while Firefox isn't, but I'm guessing that's just up to how each browser has started working with Wayland.

Sorry for the confusion.

Some upstream progress here:

  • The libinput issue for introducing hold gestures has been fixed.
  • The next step is for GTK to add support for hold gestures -- there is an open merge request for it.
  • Once the GTK support lands, supporting it becomes actionable in Firefox.

(In reply to Botond Ballo [:botond] from comment #7)

  • Once the GTK support lands, supporting it becomes actionable in Firefox.

The merge request you referenced has bin closed and merged. So GTK support has landed, correct?

(In reply to franz.srambical from comment #8)

(In reply to Botond Ballo [:botond] from comment #7)

  • Once the GTK support lands, supporting it becomes actionable in Firefox.

The merge request you referenced has bin closed and merged. So GTK support has landed, correct?

As far as I can tell, no. The GTK merge request in question is this one and its status is still open. Looking at an example change it makes (like adding a new enumerator named GDK_TOUCHPAD_HOLD to enum GdkEventType in gdk/gdkevents.h), that change is not present in the gdk/gdkevents.h file on GTK trunk.

Good news -- the GTK merge request has landed! Are changes needed in Firefox to make use of this, or will we get it for free?

Note that the MR (https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3454) only landed in GTK4 yet. It would first need to get backported to GTK3 and then IIUC we'd need to handle the new GdkEventType GDK_TOUCHPAD_HOLD.

(In reply to Robert Mader [:rmader] from comment #12)

Note that the MR (https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3454) only landed in GTK4 yet. It would first need to get backported to GTK3

Ohh good point, I didn't consider that.

and then IIUC we'd need to handle the new GdkEventType GDK_TOUCHPAD_HOLD.

It looks like that might be handled in GTK? There's a gtk_kinetic_scrolling_stop added in the patch. I can't make out for sure if it's actually wired up to GDK_TOUCHPAD_HOLD though. Also maybe Firefox overrides/ignores GTKs built in scroll functionality?

(In reply to eeshugerman from comment #13)

Also maybe Firefox overrides/ignores GTKs built in scroll functionality?

Yes, of course Firefox uses its own implementation (APZ). Firefox isn't made out of GTK widgets, the GTK window only provides a drop shadow :) and is generally just a canvas for WebRender.

(In reply to eeshugerman from comment #13)

It looks like that might be handled in GTK? There's a gtk_kinetic_scrolling_stop added in the patch. I can't make out for sure if it's actually wired up to GDK_TOUCHPAD_HOLD though. Also maybe Firefox overrides/ignores GTKs built in scroll functionality?

Not an expert here, but we generally use relatively little GTK (the actual toolkit) and a lot more of GDK (the backend/OS abstraction) in Firefox. We do already handle different kinds of GDK_SCROLL_* and GDK_TOUCHPAD_* events - this will likely be no exception.

P.S.: ah, already too late :)

Nice to see upstream progress here!

Robert and others are correct to point out that we'll need a Firefox patch to handle the new GDK_TOUCHPAD_HOLD event in our code.

Please feel free to ping the bug again when the backport to GTK3 happens, to give us a heads up to get on that.

Got it, thanks all for the info!

Please feel free to ping the bug again when the backport to GTK3 happens, to give us a heads up to get on that.

From a recent discussion on GitLab, it seems hold gestures won't be backported to GTK 3. See https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3454#note_1426517

That would mean that hold gestures won't be coming until after FF has moved to GTK 4, right?

(In reply to Christian Kaindl from comment #18)

Please feel free to ping the bug again when the backport to GTK3 happens, to give us a heads up to get on that.

From a recent discussion on GitLab, it seems hold gestures won't be backported to GTK 3. See https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3454#note_1426517

That would mean that hold gestures won't be coming until after FF has moved to GTK 4, right?

Yes, and unfortunately, based on the discussion in bug 1701123, it doesn't look like Firefox will be ported to GTK4 in the near future.

I commented on the GitLab merge request in the hope they might reconsider the GTK3 backport.

Severity: normal normal → S3 S3

(Adding "hold gestures" to the bug title to make this bug easier to find)

Summary: Kinetic scrolling on Linux ought to stop on two-finger touch → Kinetic scrolling on Linux ought to stop on two-finger touch (hold gestures)

Would it be possible for Firefox to directly poll for hold gestures from libinput rather than going through GTK3? From reading the linked ticket about porting to GTK4, it seems incredibly unlikely that FF gets ported to GTK4 any time soon (if ever).

I'm trying to finally switch from xorg-synaptics to libinput in preparation of the switch to Wayland (explicit sync is coming very soon). However, Fire continuing to scroll when it very much should not may be a dealbreaker for me, leaving me stuck on xorg-synaptics (ans thus also Xorg itself) for the foreseeable future.

(In reply to QwertyChouskie from comment #21)

Would it be possible for Firefox to directly poll for hold gestures from libinput rather than going through GTK3?

My recollection from a conversation with Emilio last year that this should be possible (though I think the idea we discussed was getting the event using Wayland APIs rather than from libinput directly).

I haven't had the time to dig into trying to implement that so far. Contributions in this area are very welcome, and I'm happy to provide Firefox-specific code pointers.

I'm trying to finally switch from xorg-synaptics to libinput in preparation of the switch to Wayland (explicit sync is coming very soon). However, Fire continuing to scroll when it very much should not may be a dealbreaker for me, leaving me stuck on xorg-synaptics (ans thus also Xorg itself) for the foreseeable future.

So hold gestures in Firefox work for you on X11? They don't work for me, I thought that's something that would only start working with Wayland (+ this bug).

So hold gestures work for you on X11? They don't work for me, I thought that's something that would only start working with Wayland (+ this bug).

The old xorg-synaptics driver implements kinetic scrolling on the driver side, apps just see it as normal scroll events. This has the advantage of making kinetic scrolling consistent across all apps/toolkits, but apparently the architecture/method wasn't sustainable for one reason or another, so libinput offloads implementing kinetic scrolling to the toolkits/apps.

(In reply to QwertyChouskie from comment #23)

So hold gestures work for you on X11? They don't work for me, I thought that's something that would only start working with Wayland (+ this bug).

The old xorg-synaptics driver implements kinetic scrolling on the driver side, apps just see it as normal scroll events. This has the advantage of making kinetic scrolling consistent across all apps/toolkits, but apparently the architecture/method wasn't sustainable for one reason or another, so libinput offloads implementing kinetic scrolling to the toolkits/apps.

Ah, you're right. I was testing libinput+X11 which does have kinetic scrolling with MOZ_USE_XINPUT2=1 but it's implemented on the Firefox side, like Wayland.

As a spare time side project, I've started to backport the GDK layer of hold gesture support (the only part that Firefox needs) from GTK4 to GTK3.

Unfortunately, I have not yet been able to test this because I have not been able to find a Wayland compositor implementation that I have access to that supports hold gestures.

On my system running PureOS 10 (Byzantium), the latest stable PureOS release:

  • in a "Plasma (Wayland)" session, the Wayland compositor is KWin, and the version available in PureOS 10 is 5.20.5, but support for hold gestures has only been added in 5.24.90
  • in a "GNOME (Wayland)" session, the Wayland compositor is mutter, and the version available in PureOS 10 is 3.38.6, but support for hold gestures has only been added in 42.0

If anyone has a suggestion for a way I could test this on my system that does not require doing something invasive (like upgrading to PureOS 11 (Crimson) which is not yet stable), please let me know.

Thanks a bunch for looking into this!

You could compile a relatively simple compositor such as sway (>=1.8). You should be able to just run it from a TTY without interrupting anything else on your system.

Also if you already run Gnome 4 it may be possible to build mutter locally from tarball (the 42.0) and then run as nested compositor:
https://fedoraproject.org/wiki/How_to_debug_Firefox_problems#Testing_different_Wayland_compositor

Flags: needinfo?(stransky)

(In reply to [:Atemu] from comment #26)

You could compile a relatively simple compositor such as sway (>=1.8).

Thanks for the suggestion. Unfortunately, after spending over an hour trying to compile sway from source and failing, most recently with this very helpful error message:

protocols/meson.build:5:21: ERROR: Could not get an internal variable and no default provided for <InternalDependency dep139988811734704: True>

I'm giving up on this line of investigation for the time being. Other suggestions are welcome. (I'm not running Gnome 4.)

Tested the patch and it crashes in gtk3, there's more place where we need to handle it. Will look at it.

Attached patch hold02.patch (obsolete) — — Splinter Review

An updated WIP patch. Doesn't produce error/crashes on Gtk3/Wayland with hold gesture supports but doesn't seem to be working, i.e. Firefox doesn't get the HOLD event. Not sure what's wrong.

Looks like the patch works and produces the hold event but it's derived to mShell only while we listen touch events on mContainer. We may miss a redirection here or so. Also I wonder how the GDK_TOUCHPAD_HOLD should be handled on Firefox side.

(In reply to Martin Stránský [:stransky] (ni? me) from comment #31)

Also I wonder how the GDK_TOUCHPAD_HOLD should be handled on Firefox side.

I started to write the Firefox patch, attaching it for reference. I didn't implement nsWindow::OnTouchpadHoldEvent yet (I was hoping to first get to a state where events are actually arriving there), but a sketch of the next steps is:

  • Add a new PanGestureInput::PanGestureType representing hold events (or maybe two, one for hold-begin and one for hold-end).
  • In nsWindow::OnTouchpadHoldEvent, create PanGestureInput events of the new type, and dispatch them with DispatchPanGesture().
  • Add logic to the AsyncPanZoomController state machine to give the new pan gesture event types their intended effects (e.g. interrupt any ongoing animation).
Assignee: nobody → botond
Assignee: botond → nobody

Working Gtk3 patch, tested with the Firefox WIP here and produces the hold events during scroll.

Attachment #9419386 - Attachment is obsolete: true
Flags: needinfo?(stransky)

As Gtk folks are not very happy with the backport let's look if we can listen to the event directly via. Wayland interface.

Flags: needinfo?(stransky)

Looks like we already handle seat_handle_capabilities so we may try to implement that on our own. That means we may also add support for wl_pointer.axis_value120 which uses similar interface (Bug 1836886).

Flags: needinfo?(stransky)
See Also: → 1836886
Flags: needinfo?(stransky)
Attached patch wip1.patch — — Splinter Review
Assignee: nobody → stransky

Botond, Bug 1915596 has implemented direct hold gesture support from Wayland display. I tested it and it works so we can proceed with hold gesture implementation here.

Flags: needinfo?(stransky)

(In reply to Martin Stránský [:stransky] (ni? me) from comment #37)

Botond, Bug 1915596 has implemented direct hold gesture support from Wayland display. I tested it and it works so we can proceed with hold gesture implementation here.

Thanks!

I'm working on getting a machine upgraded to an OS version that will allow me to test this. Needinfoing myself as as reminder to pick up the Firefox side of the implementation once I can test it.

Flags: needinfo?(botond)

Finally got my hands on a machine + software stack that actually delivers hold events. Will pick up the remaining implementation work here now that I have a setup to test it.

Assignee: stransky → botond
Flags: needinfo?(botond)

If anyone following this bug is aware of an existing Linux application that handles hold gestures, please let me know; I'd like to play around with its behaviour.

(In reply to Botond Ballo [:botond] from comment #40)

If anyone following this bug is aware of an existing Linux application that handles hold gestures, please let me know; I'd like to play around with its behaviour.

GTK4 actually implements it (grep for zwp_pointer_gestures_v1_get_hold_gesture).

(In reply to Robert Mader [:rmader] from comment #41)

GTK4 actually implements it (grep for zwp_pointer_gestures_v1_get_hold_gesture).

I'm aware of that, but I'd like an application in which to test the user-visible behaviour.

At least on Debian 12, while libgtk-4.so is present on my system, all the GTK applications I've tried so far (evince, gedit, gnote) are linked to libgtk-3.so. I'm not even getting kinetic scrolling in them.

Gedit is so GNOME 41; you want gnome-text-editor for GTK 4 (though I'm not currently on hardware that would let me verify it does hold gestures).

Ah, you bet me to it. Was about to write: Hm, most Gnome core apps - like nautilus, text-editor (replacing gedit) or gnome-builder should support it just fine AFAICS.

Thanks -- indeed I get kinetic scrolling and hold gestures in gnome-text-editor.

I've started implementing this, but I realized that we need one more piece of information beyond what is currently passed in to nsWindow::OnTouchpadHoldEvent: the event coordinates (basically, the coordinates of the cursor).

Martin, I'm wondering what approach would be best for getting those:

  • Get them from Wayland APIs in nsWaylandDisplay.cpp and pass them into nsWindow::OntouchpadHoldEvent?
  • Get them from GDK APIs in nsWindow::OnTouchpadHoldEvent?
  • Cache the cursor position in nsWindow itself (e.g. from the last call to OnMotionNotifyEvent) and use that?
Flags: needinfo?(stransky)

(In reply to Botond Ballo [:botond] from comment #46)

I've started implementing this, but I realized that we need one more piece of information beyond what is currently passed in to nsWindow::OnTouchpadHoldEvent: the event coordinates (basically, the coordinates of the cursor).

Martin, I'm wondering what approach would be best for getting those:

  • Get them from Wayland APIs in nsWaylandDisplay.cpp and pass them into nsWindow::OntouchpadHoldEvent?
  • Get them from GDK APIs in nsWindow::OnTouchpadHoldEvent?
  • Cache the cursor position in nsWindow itself (e.g. from the last call to OnMotionNotifyEvent) and use that?

Hello Botond, yes, we need that for Bug 1836886 too. I'll post a patch there which extends the stored info with coords + time stamp so we can re-use it here. Keeping needinfo to update it for hold too.

https://phabricator.services.mozilla.com/D223285 adds mLastMouseCoordinates nsWindow member.

Flags: needinfo?(stransky)

D223285 landed so the coordinates should be available now.

(In reply to Martin Stránský [:stransky] (ni? me) from comment #49)

D223285 landed so the coordinates should be available now.

Thanks! I'm planning to get back to this shortly.

(In reply to Botond Ballo [:botond] from comment #32)

I didn't implement nsWindow::OnTouchpadHoldEvent yet [...], but a sketch of the next steps is:

  • Add a new PanGestureInput::PanGestureType representing hold events (or maybe two, one for hold-begin and one for hold-end).
  • In nsWindow::OnTouchpadHoldEvent, create PanGestureInput events of the new type, and dispatch them with DispatchPanGesture().
  • Add logic to the AsyncPanZoomController state machine to give the new pan gesture event types their intended effects (e.g. interrupt any ongoing animation).

It turns out we already had PanGestureTypes which were a good fit for hold events -- PANGESTURE_MAYSTART for hold-begin, and PANGESTURE_CANCELLED for hold-end -- and appropriate handling for them inside AsyncPanZoomController. However, these event types were being dropped in InputQueue::ReceivePanGestureInput(). So the APZ part of the patch is some minor changes to that function to ensure the events get through to AsyncPanZoomController and that MAYSTART starts a new input block.

Depends on: 1836886
See Also: 1836886 →
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2dec9d90836e Handle Linux hold gestures in APZ. r=dlrobertson

https://hg.mozilla.org/mozilla-central/rev/2dec9d90836e

Status: NEW → RESOLVED
Closed: 3 months ago
status-firefox133: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch

:botond do you want to nominate this for a release note? (Process info)

Flags: needinfo?(botond)

Release Note Request (optional, but appreciated)
[Why is this notable]:
This is a Linux touchpad feature which users have been requesting since 2019. After delays related to prerequisites from other parts of the ecosystem (libinput, wayland, gtk) we are finally able to ship it.

[Suggested wording]:
Firefox now supports touchpad hold gestures on Linux. This means that kinetic (momentum) scrolling can now be interrupted by placing two fingers down on the touchpad.

relnote-firefox: --- → ?
Flags: needinfo?(botond)
No longer depends on: gtk4
See Also: → gtk4

Thanks, added to the Fx133 nightly release notes, please allow 30 minutes for the site to update.
Keeping the relnote-firefox flag as ? to keep it on the radar for inclusion in the final Fx133 release notes.

Blocks: 1926141
Whiteboard: [qa-133b-p2]
QA Whiteboard: [qa-133b-p2]
Whiteboard: [qa-133b-p2]
Regressions: 1926982
Blocks: 1929746
Blocks: 1929757
No longer blocks: 1929757
Regressions: 1929757
Blocks: 1931142

This was disabled for in beta for Fx133 via Bug 1931142
It will ride the train to Fx134 instead and should be included in the Fx134 release notes instead.

status-firefox133: fixed → disabled
status-firefox134: --- → fixed

Added to final 134 release notes

relnote-firefox: ? → 134+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: