1226522 - On-screen keyboard not displayed on machines without a rotation sensor (e.g. desktops with a touch screen)

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Vasilica Tamas, QA [:vtamas]]

Reproducible on: Firefox 45.0a1, Firefox 44.0a2 and Firefox 43 beta 5 under Windows 

Prerequisites: Tabled mode enabled

STR
1.Launch Firefox with a clean profile.
2.Tap a text field.

ER
The on-screen keyboard is automatically displayed in Tabled mode.

AR
The on-screen keyboard does not appear when a text field is focused.
- The ui.osk.debug.keyboardDisplayReason: IKPOS: Rotation sensor not found
- The ui.osk.enabled is set to true.
- See screenshot: http://i.imgur.com/VZsTIw9.jpg


Additional notes:
- This issue is reproducible on Firefox 45.0a1 (2015-11-19), Firefox 44.0a2 (2015-11-19) and Firefox 43 Beta 5 under Windows 10 64-bit.
- This testing was performed on a Dell Inspiron 5348 All-in-one with no keyboard attached.

Comment 1

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

The Dell Inspiron 5348 All-in-one is advertised as an all-in-one desktop machine, not a portable/rotatable tablet. Without having one in front of me but looking at the marketing materials, it looks like it probably doesn't have a rotation sensor and our code is showing that too.

Gijs, what do you think about dropping the rotation sensor requirement now that we know if the event came from a touch?

Comment 2

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> The Dell Inspiron 5348 All-in-one is advertised as an all-in-one desktop
> machine, not a portable/rotatable tablet. Without having one in front of me
> but looking at the marketing materials, it looks like it probably doesn't
> have a rotation sensor and our code is showing that too.
> 
> Gijs, what do you think about dropping the rotation sensor requirement now
> that we know if the event came from a touch?

I mean, this is interesting. I didn't realize it was a desktop machine. Are you saying we should also ignore the keyboard requirement in this case? Because I bet that will annoy people if they do use the keyboard and just use touch to focus things because it's easier than using a mouse...

Thanks for looking at this!

Comment 3

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

I don't know. I guess it comes down to the mind-reading and fortune-telling. Do all touches on editable elements trigger the OSK regardless of extra hardware attached? I'd say it would be best to try and model what happens in Edge and Internet Explorer.

I have a touch screen laptop with hardware keyboard permanently connected. I tried tapping in to the location bar and text entry field within a webpage on both Edge and IE and didn't get an OSK.

Vasilica, what happens on your Dell Inspiron when using Edge and IE?

Comment 4

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

Though to answer the question from comment #2, I think it would be safe to show the OSK if there is a touch but no hardware keyboard. I think we could probably do without the rotation sensor check.

Comment 5

[Vasilica Tamas, QA [:vtamas]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)

> Vasilica, what happens on your Dell Inspiron when using Edge and IE?

The OSK is successfully displayed using Edge and IE (tablet mode enabled) on Dell Inspiron.

Comment 7

[u554345]

I tested in Firefox 44.0.2

In Windows 10, I tested on-screen keyboard in a Tablet and work fine.

But I test in PC with touch screen but not a rotation sensor and fails.

In about:config I found this key:

ui.osk.debug.KeyboardDisplayReason -> IKPOS:Rotation sensor not found.

The rotation sensor should not be required.

Tho origin of this bug is in WinIMEHandler.cpp:

  if (get_rotation_state) {
    AR_STATE auto_rotation_state = AR_ENABLED;
    get_rotation_state(&auto_rotation_state);
    // If there is no auto rotation sensor or rotation is not supported in
    // the current configuration, then we can assume that this is a desktop
    // or a traditional laptop.
    if (auto_rotation_state & AR_NOSENSOR) {
      Preferences::SetString(kOskDebugReason,
                             L"IKPOS: Rotation sensor not found.");
      return false;
    } else if (auto_rotation_state & AR_NOT_SUPPORTED) {
      Preferences::SetString(kOskDebugReason,
                             L"IKPOS: Auto-rotation not supported.");
      return false;
    }
  }

Comment 10

[Loic]

From dupe: Surface Pro 3 (Win 10) using Pen

Comment 11

[Sergio]

Hi, can you say me if this bug is solved?

Thanks you.

Comment 12

[u554345]

Why is not confirmed this bug in the status?

Comment 13

[:Gijs (he/him)]

(In reply to Sergio from comment #11)
> Hi, can you say me if this bug is solved?
> 
> Thanks you.

It has not been solved yet.

(In reply to Salvador from comment #12)
> Why is not confirmed this bug in the status?

I don't understand this question. It has been confirmed ("New" rather than "unconfirmed"). We just haven't had time to fix it yet.

Comment 14

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

Attachment: Bug 1226522 - Show the on-screen keyboard from touch events even on machines lacking rotation sensors (such as touch-screen laptops).

Review commit: https://reviewboard.mozilla.org/r/53534/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53534/

Comment 15

[:Gijs (he/him)]

Comment on attachment 8753861 [details]
Bug 1226522 - Show the on-screen keyboard from touch events even on machines lacking rotation sensors (such as touch-screen laptops).

https://reviewboard.mozilla.org/r/53534/#review50280

AFAICT this will show the on-screen-keyboard even if you touch the screen on a convertible that you're using as a laptop (ie with the physical keyboard accessible and present). That seems wrong.

Can we *only* remove the rotation sensor stuff, and make the SM_CONVERTIBLESLATEMODE check conditional on the powerplatformrole not being desktop? IOW, put the last if statement you're removing in the penultimate one, change that one into `if (sPowerPlatformRole == PlatformRoleMobile || sPowerPlatformRole == PlatformRoleSlate)` ?

I'd actually also like the touch check (where we return true without checking for a physical keyboard) to only be used in that case, but I don't know if that's compatible with the world as it is. Are you able to test any of these changes on a machine yourself?

More generally all this stuff is really annoying. There are too many variants and it's impossible to test them all and make sure the code behaves properly on all types of machines when we simply don't have the hardware to actually check all those machines, especially when it's a known fact that manufacturers misconfigure their machines as well. :-\

Comment 16

[u554345]

This changes are available in any version to test?

Comment 17

[:Gijs (he/him)]

(In reply to Salvador from comment #16)
> This changes are available in any version to test?

The changes haven't been given review so they haven't landed, so no.

Comment 18

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

Comment on attachment 8753861 [details]
Bug 1226522 - Show the on-screen keyboard from touch events even on machines lacking rotation sensors (such as touch-screen laptops).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53534/diff/1-2/

Comment 19

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

(In reply to :Gijs Kruitbosch from comment #15)
> Are you able to test any
> of these changes on a machine yourself?

I have a touchscreen laptop but not a convertible or tablet at hand to test this with.

Comment 20

[:Gijs (he/him)]

Comment on attachment 8753861 [details]
Bug 1226522 - Show the on-screen keyboard from touch events even on machines lacking rotation sensors (such as touch-screen laptops).

https://reviewboard.mozilla.org/r/53534/#review60588

r+ with the below issue fixed...

::: widget/windows/WinIMEHandler.cpp:736
(Diff revision 2)
> -  if (sPowerPlatformRole != PlatformRoleMobile &&
> -      sPowerPlatformRole != PlatformRoleSlate) {
> -    Preferences::SetString(kOskDebugReason, L"IKPOS: PlatformRole is neither Mobile nor Slate.");
> -    return false;
> +  if ((sPowerPlatformRole == PlatformRoleMobile ||
> +       sPowerPlatformRole == PlatformRoleSlate) &&
> +      ::GetSystemMetrics(SM_CONVERTIBLESLATEMODE) == 0 &&
> +      sLastContextActionCause == InputContextAction::CAUSE_TOUCH) {
> +      Preferences::SetString(kOskDebugReason, L"IKPOS: Mobile/Slate Platform role, in slate mode with touch event.");
> +      return true;
> -  }
> +    }
> -
> -  // Likewise, if the tablet/mobile isn't in "slate" mode, we should bail:
> -  if (::GetSystemMetrics(SM_CONVERTIBLESLATEMODE) != 0) {
> -    Preferences::SetString(kOskDebugReason, L"IKPOS: ConvertibleSlateMode is non-zero");
> -    return false;
>    }

This patch causes this file to fail to compile because there are two closing braces here when there should be only one... The `return true` should also be indented 2 spaces in comparison to the `if` before it, not 4.

Comment 21

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

Comment on attachment 8753861 [details]
Bug 1226522 - Show the on-screen keyboard from touch events even on machines lacking rotation sensors (such as touch-screen laptops).

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53534/diff/2-3/

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/621f32c868bb
Show the on-screen keyboard from touch events even on machines lacking rotation sensors (such as touch-screen laptops). r=Gijs

Comment 23

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

https://hg.mozilla.org/mozilla-central/rev/621f32c868bb

Comment 24

[Cornel Ionce [:noni] [Hubs QA]]

Confirming that the OSK is properly displayed on Windows 10x64 using the str from the description on latest Nightly, build ID: 20160727030230.

Comment 25

[u554345]

Confirmated, OSK in the Nightly build 20160727030230 work fine using Windows 10x32 with a touch screen without rotation sensor.