GNOME Bugzilla – Bug 788188
On-screen keyboard shows up on first touch even when touch does not activate a text-field
Last modified: 2018-02-06 00:48:53 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!
*** Bug 786284 has been marked as a duplicate of this bug. ***
*** Bug 788616 has been marked as a duplicate of this bug. ***
Carrying over from my report, I have experienced this in X11 and the Wayland session.
Created attachment 361539 [details] [review] keyboard: Minor cleanup _syncEnabled() will call _setupKeyboard() if necessary, so no need to call it explicitly before.
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.
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.
Big disclaimer: I don't have any touch hardware to test, so those patches are pretty much just guesswork.
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.
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?
There weren't any changes in keyboard.js since 3.25.91, and no changes at all that affect these patches.
Alright. I should said 3.26.2 or before. Thank you.
Comment on attachment 361539 [details] [review] keyboard: Minor cleanup LGTM
Comment on attachment 361540 [details] [review] keyboard: Split enabled setting from enabled state Nice :)
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.
(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)
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.
(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).
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
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.
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.
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
(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?
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.
The last patch did land with the OSK rewrite, so re-closing.