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 788188 - On-screen keyboard shows up on first touch even when touch does not activate a text-field
On-screen keyboard shows up on first touch even when touch does not activate ...
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: keyboard
3.26.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 786284 788616 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-09-26 12:31 UTC by Atri
Modified: 2018-02-06 00:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
keyboard: Minor cleanup (1015 bytes, patch)
2017-10-13 16:22 UTC, Florian Müllner
committed Details | Review
keyboard: Split enabled setting from enabled state (3.12 KB, patch)
2017-10-13 16:22 UTC, Florian Müllner
committed Details | Review
keyboard: Don't pop up on touch events (1.75 KB, patch)
2017-10-13 16:22 UTC, Florian Müllner
committed Details | Review
keyboard: Fix OSK hiding on non-touchscreen input (1.42 KB, patch)
2018-01-18 18:00 UTC, Carlos Garnacho
none Details | Review
keyboard: Restore intended OSK visibility behavior (1.85 KB, patch)
2018-01-19 11:34 UTC, Carlos Garnacho
none Details | Review

Description Atri 2017-09-26 12:31:26 UTC
After using the keyboard (or bluetooth stylus), the first touch gesture I make on the touchscreen always brings up the on-screen keyboard, irrespective of whether the touch activates a text-field. This is annoying, for instance when I am using the touchscreen on the maps app (which is much more convenient than from the touchpad), or when I am using the stylus and want to use a touch-screen gesture to scroll to the next page. This is all on X11, fwiw.

I have seen bug 742246, but the present bug is likely different, and since I have been using gnome 3 with a touchscreen laptop for a few years now, looks like a regression introduced with version 3.26.

Happy to attach any requested logs, or more information. Thanks!
Comment 1 Strangiato 2017-09-27 04:37:08 UTC
*** Bug 786284 has been marked as a duplicate of this bug. ***
Comment 2 Florian Müllner 2017-10-06 20:15:14 UTC
*** Bug 788616 has been marked as a duplicate of this bug. ***
Comment 3 parker.l.reed@gmail.com 2017-10-06 21:22:33 UTC
Carrying over from my report, I have experienced this in X11 and the Wayland session.
Comment 4 Florian Müllner 2017-10-13 16:22:15 UTC
Created attachment 361539 [details] [review]
keyboard: Minor cleanup

_syncEnabled() will call _setupKeyboard() if necessary, so no need
to call it explicitly before.
Comment 5 Florian Müllner 2017-10-13 16:22:25 UTC
Created attachment 361540 [details] [review]
keyboard: Split enabled setting from enabled state

We enable the keyboard when it is either enabled explicitly via
a11y settings or when using a touch device. We'll soon want to
special-case changes to the GSettings, so track its value in a
dedicated property.
Comment 6 Florian Müllner 2017-10-13 16:22:35 UTC
Created attachment 361541 [details] [review]
keyboard: Don't pop up on touch events

We want touch events to enable the keyboard and focus tracking, but
not to actually show it right away. Implement that behavior by only
changing the visibility of the keyboard when triggered by a GSettings
change.
Comment 7 Florian Müllner 2017-10-13 16:23:46 UTC
Big disclaimer: I don't have any touch hardware to test, so those patches are pretty much just guesswork.
Comment 8 Atri 2017-10-14 21:16:49 UTC
Hi Florian, many thanks for the patch. I tested it on my tablet (patch applied to gnome-shell 3.26.1), and it fixed the issue completely. Great work, much appreciated.
Comment 9 Luciano Santos 2017-11-04 19:55:53 UTC
Hey Müllner, what's going on! This patches of yours, they have a total of 9 hunks of changes. With the version 3.26.2 I saw that keyboard.js had some changes, what hunks can I drop?
Comment 10 Florian Müllner 2017-11-04 20:08:01 UTC
There weren't any changes in keyboard.js since 3.25.91, and no changes at all that affect these patches.
Comment 11 Luciano Santos 2017-11-04 23:23:38 UTC
Alright. I should said 3.26.2 or before. Thank you.
Comment 12 Carlos Garnacho 2018-01-16 12:36:31 UTC
Comment on attachment 361539 [details] [review]
keyboard: Minor cleanup

LGTM
Comment 13 Carlos Garnacho 2018-01-16 12:37:49 UTC
Comment on attachment 361540 [details] [review]
keyboard: Split enabled setting from enabled state

Nice :)
Comment 14 Carlos Garnacho 2018-01-16 12:40:31 UTC
Review of attachment 361541 [details] [review]:

Other than a minor nit, looks good to me.

::: js/ui/keyboard.js
@@ +321,1 @@
             Main.layoutManager.hideKeyboard(true);

I think this is unnecessary if you just called _setupKeyboard(), visibility is off by default.
Comment 15 Florian Müllner 2018-01-16 15:06:35 UTC
(In reply to Carlos Garnacho from comment #14)
> ::: js/ui/keyboard.js
> @@ +321,1 @@
>              Main.layoutManager.hideKeyboard(true);
> 
> I think this is unnecessary if you just called _setupKeyboard(), visibility
> is off by default.

In GTK+, not in Clutter - actors are visible by default, so after calling _setupKeyboard(), the visibility depends entirely on whether Main.layoutManager.keyboardBox itself is visible or not.

(But then I haven't really tested this due to missing touch hardware, though the previous code treated a call to _setupKeyboard() as showing it)
Comment 16 Carlos Garnacho 2018-01-16 15:48:21 UTC
But AFAICS keyboardBox is hidden early on startup:
https://git.gnome.org//browse/gnome-shell/tree/js/ui/layout.js#n636

If we're creating the widgetry, I think it's safe to assume it wasn't ever shown before? Anyway, it's a pretty harmless thing to do.
Comment 17 Florian Müllner 2018-01-16 16:19:47 UTC
(In reply to Carlos Garnacho from comment #16)
> If we're creating the widgetry, I think it's safe to assume it wasn't ever
> shown before?

I have no idea how relevant it is in practice, but we call _destroyKeyboard() (without hiding the parent container) when the keyboard type changes. But I can leave out the call for now, and we'll see if there's an issue in practice (and after landing the extensive changes from your branch).
Comment 18 Florian Müllner 2018-01-16 18:29:17 UTC
Attachment 361539 [details] pushed as d5f081a - keyboard: Minor cleanup
Attachment 361540 [details] pushed as cf23490 - keyboard: Split enabled setting from enabled state
Attachment 361541 [details] pushed as 705915c - keyboard: Don't pop up on touch events
Comment 19 Carlos Garnacho 2018-01-18 17:59:55 UTC
Reopening. I missed a behavioral change, the OSK is no longer hidden when input from mice/keyboards happens after using the touchscreen. Attaching a patch for this.
Comment 20 Carlos Garnacho 2018-01-18 18:00:39 UTC
Created attachment 367027 [details] [review]
keyboard: Fix OSK hiding on non-touchscreen input

This logic was left depending on the a11y setting only, but should also
react to the last input device.
Comment 21 Florian Müllner 2018-01-18 22:43:10 UTC
Review of attachment 367027 [details] [review]:

::: js/ui/keyboard.js
@@ +320,3 @@
             this._setupKeyboard();
 
+        if (this._enabled && !wasEnabled)

Mmmh, doesn't this mean that we show the keyboard again after using the touchscreen, independent from focus? (That is, the problem we were trying to fix in the first place)

@@ +323,2 @@
             Main.layoutManager.showKeyboard();
+        else if (!this._enabled && wasEnabled)

This on the other hand makes sense to me
Comment 22 Carlos Garnacho 2018-01-18 23:20:47 UTC
(In reply to Florian Müllner from comment #21)
> Review of attachment 367027 [details] [review] [review]:
> 
> ::: js/ui/keyboard.js
> @@ +320,3 @@
>              this._setupKeyboard();
>  
> +        if (this._enabled && !wasEnabled)
> 
> Mmmh, doesn't this mean that we show the keyboard again after using the
> touchscreen, independent from focus? (That is, the problem we were trying to
> fix in the first place)

Oh right. I've got this fixed later on osk-cldr (already rebased on top of this) by checking the ClutterInputMethod focus. I can maybe add a check for this._currentAccessible here and rebase again?
Comment 23 Carlos Garnacho 2018-01-19 11:34:17 UTC
Created attachment 367077 [details] [review]
keyboard: Restore intended OSK visibility behavior

Getting the necessary "setting enabled, or input from touchscreen"
conditions to have the OSK shown are not enough on the lack of a
current focus. As we are setting up the caret tracker here, wait for
the focus in event before showing the keyboard.

This fixes 2 issues, with the setting disabled it became really hard
to get the OSK hidden on eg. touchscreen->pointer device switches,
as visibility only depended on the a11y setting here. And secondly,
enabling the setting would always end up with the OSK being shown
regardless of focus, while it should stay hidden if there's no text
edition.
Comment 24 Florian Müllner 2018-02-06 00:48:53 UTC
The last patch did land with the OSK rewrite, so re-closing.