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 704050 - When using natural scrolling, lockscreen unlock scroll direction is reversed
When using natural scrolling, lockscreen unlock scroll direction is reversed
Status: RESOLVED OBSOLETE
Product: gnome-shell
Classification: Core
Component: lock-screen
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 684570 703919 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-07-11 19:17 UTC by James Bates
Modified: 2021-07-05 14:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch file which ensures that the scroll direction to unlock the lockscreen is always up, regardless of whether natural scrolling is enabled or not. (1.32 KB, patch)
2013-07-11 19:17 UTC, James Bates
reviewed Details | Review
gnome-shell natural scrolling lockscreen fix (reviewed by Giovanni Campagna) (1.93 KB, patch)
2013-07-14 15:09 UTC, James Bates
needs-work Details | Review
natural-scrolling-lockscreen-patch (2.07 KB, patch)
2013-07-14 21:39 UTC, James Bates
none Details | Review

Description James Bates 2013-07-11 19:17:38 UTC
Created attachment 248961 [details] [review]
patch file which ensures that the scroll direction to unlock the lockscreen is always up, regardless of whether natural scrolling is enabled or not.

If you set your system to use "Touhcpad natural scrolling", then to unlock the lockscreen it is currently necessary to scroll - down - which is quite counter-intuitive.

It would seem more intuitive that, to unlock the lockscreen, one should - always - scroll up (as the arrows indicate), regardless whether one has set "natural scrolling" or not.

It is possible to patch the file js/ui/screenShield.js to function precisely in that way, as indicated in the attached patch.
Comment 1 Giovanni Campagna 2013-07-13 15:54:07 UTC
Review of attachment 248961 [details] [review]:

::: js/ui/screenShield.js.orig
@@ +626,3 @@
             return false;
 
+        let touchpadSettings = new Gio.Settings({ schema: TOUCHPAD_SCHEMA });

If you create the GSettings schema in the constructor, we can save an AddMatch call every time.

@@ +631,3 @@
         let delta = 0;
+        if ((naturalScroll && event.get_scroll_direction() == Clutter.ScrollDirection.DOWN)
+                || (!naturalScroll && event.get_scroll_direction() == Clutter.ScrollDirection.UP))

Bad indentation, the second line should be aligned with the parenthesis.

Also, I would go with:
let multiplier, direction;
if (naturalScroll) {
    multiplier = -1;
    direction = Clutter.ScrollDirection.DOWN;
} else {
    ...
}

And then use those to compare the event values.
Comment 2 Giovanni Campagna 2013-07-13 15:54:24 UTC
Oh and by the way, thanks for the patch!
Comment 3 Giovanni Campagna 2013-07-14 13:00:13 UTC
*** Bug 703919 has been marked as a duplicate of this bug. ***
Comment 4 James Bates 2013-07-14 14:11:19 UTC
Thank you for the correction suggestions. Do you need a corrected patch, or can you build it yourself in the way suggested?
Comment 5 Giovanni Campagna 2013-07-14 14:24:13 UTC
(In reply to comment #4)
> Thank you for the correction suggestions. Do you need a corrected patch, or can
> you build it yourself in the way suggested?

Well, it depends on you. If you want to be a GNOME contributor, an improved patch is appreciated :)
Also, you should use git-format-patch to make it, and add a commit message on top of it.
Comment 6 James Bates 2013-07-14 15:09:11 UTC
Created attachment 249125 [details] [review]
gnome-shell natural scrolling lockscreen fix (reviewed by Giovanni Campagna)

Incorporated review suggestions by Giovanni Campagna
Comment 7 Giovanni Campagna 2013-07-14 15:26:51 UTC
Review of attachment 249125 [details] [review]:

Looks good!

Do you have a git account or do you want me to push it?
Comment 8 Florian Müllner 2013-07-14 15:28:37 UTC
Review of attachment 249125 [details] [review]:

The patch is wrong. The scroll direction is reversed when 'natural-scroll' is true AND the device is a touchpad (see https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/mouse/gsd-mouse-manager.c#n983).
Comment 9 Jasper St. Pierre (not reading bugmail) 2013-07-14 15:35:20 UTC
So, I assume we'll need to write an XGetDeviceProperty wrapper? :(
Comment 10 Giovanni Campagna 2013-07-14 15:40:55 UTC
(In reply to comment #8)
> Review of attachment 249125 [details] [review]:
> 
> The patch is wrong. The scroll direction is reversed when 'natural-scroll' is
> true AND the device is a touchpad (see
> https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/mouse/gsd-mouse-manager.c#n983).

Oh ouch....

(In reply to comment #9)
> So, I assume we'll need to write an XGetDeviceProperty wrapper? :(

clutter_input_device_get_device_type() should be enough, I suppose.
Comment 11 Jasper St. Pierre (not reading bugmail) 2013-07-14 15:46:18 UTC
(In reply to comment #10)
> clutter_input_device_get_device_type() should be enough, I suppose.

Except if it's a touchpad device without the device property "Synaptics Scrolling Distance".
Comment 12 Giovanni Campagna 2013-07-14 15:51:49 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > clutter_input_device_get_device_type() should be enough, I suppose.
> 
> Except if it's a touchpad device without the device property "Synaptics
> Scrolling Distance".

Well, that would trigger a g_warning, if I read the code correctly, so it's not an expected case.
Comment 13 Jasper St. Pierre (not reading bugmail) 2013-07-14 16:37:07 UTC
You read incorrectly. If the property doesn't exist on the device, the XGetDeviceProperty will return True and not raise an error, but return None for act_type and 0 for nitems.

The error is only taken if the property exists and is valid, but can't be changed.
Comment 14 James Bates 2013-07-14 21:38:08 UTC
On my setup (MacbookPro6,1 using nouveau graphics and bcm5974 touchpad, which to X looks like a 'synaptics' as far as I know, and also a bluetooth Microsoft "Notebook Mouse 5000" with a scrollwheel), clutter_event_get_device_type() always returns CLUTTER_POINTER_DEVICE, no matter whether I use the trackpad or the mouse.

However clutter_event_get_source_device() returns a device for which clutter_iput_device_get_device_type() - does - return a CLUTTER_TOUCHPAD_DEVICE for a touchpad device and CLUTTER_POINTER_DEVICE for the mouse.

I've modified the patch to take it into account. It now works correctly in all circumstances: with natural scrolling enabled or without, using either the touchpad or the mouse with a scrollwheel. I would imagine that many users who use "natural scrolling" are mac users, or ex-mac users since (as far as I know) Apple first introduced the concept... In that sense, my machine is probably quite typical?
Comment 15 James Bates 2013-07-14 21:39:29 UTC
Created attachment 249144 [details] [review]
natural-scrolling-lockscreen-patch

Fix scroll direction on Lockscreen for users with "natural scrolling" aka "content sticks to fingers" enabled.
Comment 16 James Bates 2013-07-14 21:40:39 UTC
As for git, no, I have no public git account where you can come and pull, so I'd be much obliged, if you decide to use the patch, if someone could insert it for me ;)
Comment 17 David Strauss 2013-07-14 22:18:56 UTC
I see this discussion around natural/sticky scrolling for trackpads versus mice. On OS X systems, the "natural" scrolling affects both types of input devices. Is there a reason GNOME distinguishes? It's probably out of the scope of this issue, but it would definitely simplify the fix here if the lock screen didn't have to know the device type initiating the scrolling action.

In any case, it's a break in abstraction to have the lock screen worry about a combination of sticky scrolling plus input device type. It would be best to expose  consistent *kinetic direction* double-finger grabbing events for use in situations like this.
Comment 18 Jasper St. Pierre (not reading bugmail) 2013-07-15 04:45:37 UTC
Because we don't have a gesture recognition API yet -- all we have two-finger scrolling events from the X server, and we'll make do with what we have.
Comment 19 James Bates 2013-07-17 10:47:16 UTC
Some additional information:

I installed the exact same software configuration (Same kernel; xorg-server and gnome version) onto completely different hardware: a Dell Inspiron 1501 laptop from 2004, which has an actual synaptics touchpad:

james@dell:~$ dmesg | grep synaptics
[    1.467547] psmouse serio1: synaptics: Touchpad model: 1, fw: 6.3, id: 0x180b1, caps: 0xa04713/0x200000/0x0, board id: 3655, fw id: 260880

Now two-finger scrolling and natural scrolling work just fine when set-up in gnome-control-center (i.e. the code in gsd-mouse-manager.c correctly detects that it is a touchpad, in particular line 821 functions correctly. device_is_touchpad() comes from gdk I believe...)

However on this machine clutter_event_get_source_device() -> clutter_input_device->get_device_type() always returns CLUTTER_POINTER_DEVICE. The same goes for clutter_event_get_input_device_type() (as was already the case on the mac). So it seems that clutter determines the device type (in this case wrongly) using some other algorithm than the one from gdk, which gnome-settings-daemon uses, causing the discrepancy.

So I suspect it will be necessary to compare the gdk and the clutter implementations, and possibly align them?
Comment 20 Allan Day 2013-08-20 09:49:44 UTC
*** Bug 684570 has been marked as a duplicate of this bug. ***
Comment 21 Greg K Nicholson 2013-08-23 22:03:19 UTC
Re comment 18:

Why not allow scrolling in *either* vertical direction to unlock the shield?

We wouldn't have to worry about the input device, or about which direction makes more sense for any given type of input device. Would this be harmful in some way I haven't thought of?
Comment 22 GNOME Infrastructure Team 2021-07-05 14:37:55 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of  gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/

Thank you for your understanding and your help.