1192573 - When using a touchscreen device, on screen keyboard appearing even when a physical keyboard is present

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Josh Tumath]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150808030207

Steps to reproduce:

1. Use a touchscreen laptop (I have a Dell XPS 12).
2. Click on any text box in the chrome or content with your mouse


Actual results:

The on screen keyboard will pop up


Expected results:

Windows 8:
The on screen keyboard should not pop up unless I have touched the textbox with a touch input.

Windows 10:
The on screen keyboard should only pop up in tablet mode.

Comment 1

[Thomas]

The problem is related to ui.osk.on_screen_Keyboard_path
This entry in about:config is written in bold letters so the Path in there is not Standard.
Did a Reset to default but As soon You Enter a Text Field the Path to the on Screen Keyboard exe is Back and the on screen Keyboard Pops up.
Quite annoying

Furthermore:
Even if You turn off the on Screen Keyboard Service in Windows This Happens.

Temporary Solution: Enter something Else instead of a Path. I did so with a - and The Problem is Gone

Comment 2

[Thomas]

That The Standard entry (Empty) gets overwritten all the time is a second Bug IMHO

Comment 3

[Thomas]

Content of the field is: "C:\Program Files\Common Files\microsoft shared\ink\TabTip.exe"

Comment 4

[Josh Tumath]

(In reply to Thomas from comment #2)
> That The Standard entry (Empty) gets overwritten all the time is a second
> Bug IMHO

No, that's not a bug at all. The code is simply checking for the location of the TapTip executable in order to manually launch it itself. The path isn't guaranteed to always be in the same place, which is why it is stored in a path. Removing the pref would only temporarily prevent the keyboard from appearing at all, which is not what we want. We want the keyboard to appear in particular circumstances, rather than all the time.

Comment 5

[Thomas]

But the standard value of this files is empty. So either the standard value should be the path I quoted or this value should stay empty.

Comment 6

[Jim Mathies [:jimm]]

Front end code needs to decide if this is touch related input, or better yet, we should build the detection into widget so web pages and addons can't abuse the soft keyboard. This is the way the soft keyboard is supposed to work in win8 and up.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

I will put up a patch here to limit the on-screen keyboard to Windows 10 tablet mode. We can land this now and then work on getting broader support later.

By the way, if you want to disable the feature you should set `ui.osk.enabled` to false in about:config.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I did some minor cleanup while in here. The only necessary change is the few lines related to GetIsInTabletMode (and the related IsWin8OrLater -> IsWin10OrLater changes).

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8645765 [details] [diff] [review]
Patch

>+#include "WindowsUIUtils.h"

Don't you really need nsIWindowUIUtils.h, isn't it?

>@@ -209,21 +210,21 @@ IMEHandler::NotifyIME(nsWindow* aWindow,
>         return NS_OK;
>       case NOTIFY_IME_OF_TEXT_CHANGE:
>         return TSFTextStore::OnTextChange(aIMENotification);
>       case NOTIFY_IME_OF_FOCUS: {
>         IMMHandler::OnFocusChange(true, aWindow);
>         nsresult rv =
>           TSFTextStore::OnFocusChange(true, aWindow,
>                                       aWindow->GetInputContext());
>-        IMEHandler::MaybeShowOnScreenKeyboard();
>+        IMEHandler::ShowOnScreenKeyboard();
>         return rv;
>       }
>       case NOTIFY_IME_OF_BLUR:
>-        IMEHandler::MaybeDismissOnScreenKeyboard();
>+        IMEHandler::DismissOnScreenKeyboard();

I don't like these changes. "Maybe" sounds better since in most environments, VKB isn't show/closed by these methods.

>@@ -263,20 +264,20 @@ IMEHandler::NotifyIME(nsWindow* aWindow,
>       return NS_OK;
>     case NOTIFY_IME_OF_SELECTION_CHANGE:
>       IMMHandler::OnSelectionChange(aWindow, aIMENotification, true);
>       return NS_OK;
>     case NOTIFY_IME_OF_MOUSE_BUTTON_EVENT:
>       return IMMHandler::OnMouseButtonEvent(aWindow, aIMENotification);
>     case NOTIFY_IME_OF_FOCUS:
>       IMMHandler::OnFocusChange(true, aWindow);
>-      IMEHandler::MaybeShowOnScreenKeyboard();
>+      IMEHandler::ShowOnScreenKeyboard();
>       return NS_OK;
>     case NOTIFY_IME_OF_BLUR:
>-      IMEHandler::MaybeDismissOnScreenKeyboard();
>+      IMEHandler::DismissOnScreenKeyboard();

Same above.

>-void
>-IMEHandler::MaybeShowOnScreenKeyboard()
>-{
>-  if (sPluginHasFocus) {
>-    return;
>-  }
>-  IMEHandler::ShowOnScreenKeyboard();
>-}

I think that this method should have the first if block of ShowOnScreenKeyboard() too. And new check which you are adding should be here too.

>-// static
>-void
>-IMEHandler::MaybeDismissOnScreenKeyboard()
>-{
>-  if (sPluginHasFocus) {
>-    return;
>-  }

Similarly, the first check of DismissOnScreenKeyboard() should be here.

>-  bool result = false;
>   // Query for all the keyboard devices.
>   HDEVINFO device_info =
>     ::SetupDiGetClassDevs(&KEYBOARD_CLASS_GUID, nullptr,
>                           nullptr, DIGCF_PRESENT);
>   if (device_info == INVALID_HANDLE_VALUE) {
>-    return result;
>+    return false;
>   }
> 
>   // Enumerate all keyboards and look for ACPI\PNP and HID\VID devices. If
>   // the count is more than 1 we assume that a keyboard is present. This is
>   // under the assumption that there will always be one keyboard device.
>   for (DWORD i = 0;; ++i) {
>     SP_DEVINFO_DATA device_info_data = { 0 };
>     device_info_data.cbSize = sizeof(device_info_data);
>@@ -670,35 +650,51 @@ IMEHandler::IsKeyboardPresentOnSlate()
>       if (IMEHandler::WStringStartsWithCaseInsensitive(device_id,
>                                                        L"ACPI") ||
>           IMEHandler::WStringStartsWithCaseInsensitive(device_id,
>                                                        L"HID\\VID")) {
>         // The heuristic we are using is to check the count of keyboards and
>         // return true if the API's report one or more keyboards. Please note
>         // that this will break for non keyboard devices which expose a
>         // keyboard PDO.
>-        result = true;
>+        return true;
>       }
>     }
>   }
>-  return result;
>+  return false;
> }

Good!

I'd like to check the new patch, so, temporarily -'ing.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

Ok, I added back the Maybe* functions. I also added a pref to disable the "require_tablet_mode" to help QA and others test this that may not have a tablet at hand.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8646966 [details] [diff] [review]
Patch v2

>+  // Tablet Mode is only supported on Windows 10 and higher.
>+  // When touch-event detection within IME is better supported
>+  // this check may be removed, and ShowOnScreenKeyboard can
>+  // run on Windows 8 and higher (adjusting the IsWin10OrLater
>+  // guard above and within MaybeDismissOnScreenKeyboard).
>+  nsCOMPtr<nsIWindowsUIUtils>
>+    uiUtils(do_GetService("@mozilla.org/windows-ui-utils;1"));
>+  if (uiUtils) {
>+    bool isInTabletMode = false;
>+    uiUtils->GetInTabletMode(&isInTabletMode);
>+    if (!isInTabletMode &&
>+        Preferences::GetBool(kOskRequireTabletMode, true)) {

Well, if Preferences::GetBool(kOskRequireTabletMode, true) is false, the result is always ignored. So, following code looks reasonable to me:

if (Preferences::GetBool(kOskRequireTabletMode, true)) {
  nsCOMPtr<nsIWindowsUIUtils>
    uiUtils(do_GetService("@mozilla.org/windows-ui-utils;1"));
  if (uiUtils) {
    ...

But this could be more expensive than your code (not sure). I think that for now, you should use this style temporarily and will use Preferences::AddBoolVarCache() for all prefs. Then, this style becomes cheaper and the diff becomes simplest. (Because the prefs may be checked every focus change on Win10, therefore, using VarCache improves the runtime cost.)

With the change and filing a bug to use Preferences::AddBoolVarCache(), r=masayuki

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c21b91a01cce

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c21b91a01cce

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8646966 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: regression from bug 1007063
[User impact if declined]: on-screen keyboard will show up when not needed
[Describe test coverage new/current, TreeHerder]: manual testing, uses tablet-mode detection which has been in tree for a while now
[Risks and why]: low risk, reduces the impact of the on-screen keyboard
[String/UUID change made/needed]: none

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8646966 [details] [diff] [review]
Patch v2

Regression, beginning of the aurora cycle, taking it.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/90b307848817

Comment 17

[bvanoudtshoorn]

With the update to Dev Edition 43 (2015-09-29), this problem has resurfaced -- focussing any text-entry field in both the chrome and the content causes the on-screen keyboard to display. This includes focussing by tabbing to the field with the physical keyboard.

I am not running in tablet mode.

Comment 18

[bvanoudtshoorn]

I should also note (I don't know if it's related or not) that if I have Gmail open on one screen to reply to an email, and I repeatedly switch focus between the recipients list and the main editor, it's a bit of a lottery as to which screen the OSK appears on.

Additional information on my set up, if it helps:
- Windows 10
- Surface Pro 3 i7
- Attached to a dock, with the keyboard plugged into the dock
- Keyboard is an Apple full-size aluminium keyboard
- `about:buildconfig` says that my version was built from https://hg.mozilla.org/releases/mozilla-aurora/rev/e7b899c144c9cac4ddc6c574c270db75e811394c and targets i686.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to bvanoudtshoorn from comment #17)
> With the update to Dev Edition 43 (2015-09-29), this problem has resurfaced
> -- focussing any text-entry field in both the chrome and the content causes
> the on-screen keyboard to display. This includes focussing by tabbing to the
> field with the physical keyboard.
> 
> I am not running in tablet mode.

(In reply to bvanoudtshoorn from comment #18)
> - Attached to a dock, with the keyboard plugged into the dock
> - Keyboard is an Apple full-size aluminium keyboard
> - `about:buildconfig` says that my version was built from
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> e7b899c144c9cac4ddc6c574c270db75e811394c and targets i686.

Thanks for commenting here, OK here's a few steps that I'd like you to check and post the results here.

1) Go to about:config and look up the value for the `ui.osk.detect_physical_keyboard` preference. Is it set to "false"? If it is set to "false", set it to "true" and move number 2.

1a) If the result from step 1 was that the setting was set to "true", then there is an issue in finding that your computer is docked. We call ::GetSystemMetrics(SM_SYSTEMDOCKED) and compare the result with 0. According to MSDN, if it returns 0 then it means that the system is undocked. Is there some setting or toggle within Windows that can confirm that the OS thinks it's docked?

2) Do you have "Automatically show the touch keyboard in windowed apps when there's no keyboard attached to your device" set to On? You can follow the steps here to see, http://www.windowscentral.com/auto-display-touch-keyboard-windows-10-desktop-mode

If that setting is on, then the requirement for Tablet Mode is disabled. This is the only change in Firefox 43, and is likely your issue, but step 1 is also a problem that will need to be investigated.

Comment 20

[bvanoudtshoorn]

Thanks for the quick response!

1) `ui.osk.detect_physical_keyboard` is at its default value, `true`.

1a) There's nothing specific that I can point to here. Going through Device Manager, though, there are a couple of devices which become available when docked -- like "Microsoft Docking Station Audio Device", a second external monitor, and a couple of USB hubs. The "Tablet Mode" option is also disabled in the Action Centre, as is the Battery Saver option.

2. Yes, this option is enabled -- in the Devices section of the Settings app, "Automatically show the touch keyboard in windowed apps when there's no keyboard attached to your device" is set to "on". Changing this to "off" resolves the issue.

Turning this off does mean that I either have to swap to tablet mode (which I dislike) or tap the keyboard button (which isn't really a problem), so I'm happy to leave it in the "off" state. So my personal issue is resolved, but I would still suggest that this behaviour isn't something that should make it through to general users. I'm more than happy to download nightly builds to test any changes to this functionality if that would be useful.

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

Thanks for going through those steps. Yeah it would be nice if you could keep that option from step 2. It wouldn't be a problem if the docking-mode detection worked correctly.

Would you like to file a new bug stating that our code isn't correctly detecting that your Surface Pro is docked?

Comment 22

[bvanoudtshoorn]

Thanks Jared -- I've filed this issue as https://bugzilla.mozilla.org/show_bug.cgi?id=1211805.

Comment 23

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 42.0a1 (2015-08-08) under Windows 8.1 64-bit using a Dell Xps 12.

Verified fixed on Firefox 43 beta 8 (20151201152349) and Firefox 42 RC (20151029151421) under Windows 8.1 64-bit and Windows 10 64-bit using a Dell Xps 12.