1221947 - On-screen keyboard is not displayed on a device with hardware keyboard even when keyboard is not available

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Vasilica Tamas, QA [:vtamas]]

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

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

ER
The on-screen keyboard is automatically displayed.

AR
[1] The on-screen keyboard does not appear when a text box is focused.
[2] Exception: The keyboard appears only for a password field when table mode is enabled and e10s is disabled.
*The ui.osk.debug.keyboardDisplayReason: IKPOS: Keyboard presence confirmed.
*The ui.osk.enabled is set to true.


Additional notes:
- This issue is reproducible on Firefox 45.0a1 (2015-11-04), Firefox 44.0a2 (2015-11-04) and Firefox 43 Beta 1 build 2 under Windows 10 64-bit.
- This issue reproduces in both on/off Tablet Mode ways.
- This testing was performed on a Dell XPS 12 which has a hardware keyboard.

Comment 1

[:Gijs (he/him)]

> Can you post a screenshot of using this tool:
> 
> http://www.nirsoft.net/utils/usb_devices_view.html
> 
> on the XPS when the machine is flipped so you can't use the keyboard? Can you show the
> difference between that state and the one where the keyboard *is* usable (ie machine isn't flipped) ?

Comment 2

[Vasilica Tamas, QA [:vtamas]]

Here are the screenshots, but I see no difference between them:

Flipped screen (unusable keyboard): http://i.imgur.com/4qVUgCY.png
Vertical screen (usable keyboard): http://i.imgur.com/0bMB7Hz.jpg

Comment 3

[:Gijs (he/him)]

So both here and in bug 1221961 and for the last comment in bug 1007063, we basically need a way to distinguish the "attached but unusable" keyboard from a USB keyboard.

Chrome doesn't make this distinction and always listens to the device "orientation", so if you use a USB keyboard attached to a tablet, you get a soft keyboard when touching a field. However, it *only* shows the keyboard if a field is focused via touch. We don't currently have that information in our IME handler.

One thing we could do would be whitelisting device IDs for things like the surface pro touch cover and the builtin keyboard on these dell xps machines... but I'm worried that that won't scale very well. I haven't found a generic way to detect these situations - the device view screenshots in this and the surface bug don't show the usb device as disabled or something like that.

Another thing we could do would be trying to ensure we know the source of the ime focus thing so we can get parity with chrome. That'd be a regression in some respects, but better in others.

Jared/Jim, how do you feel about the situation here? Can we think of other ways to tackle the distinction between an external "really-"USB-connected keyboard and the type cover / disabled-convertible-keyboard things?

Comment 4

[Jim Mathies [:jimm]]

I think we would like to have both to do this right. I'd bet we can find a way to update IMEStateManager::NotifyIME to pass event source information through, it's available via gecko/dom events. If we don't want to tackle that here, lets get a bug filed on it.

The enabled keyboard detection is probably tricky. The winrt apis manage to do this reliably though so there's probably a combination that works. There are apis for querying a list of devices using the raw input apis - 

https://msdn.microsoft.com/en-us/library/windows/desktop/ff468896%28v=vs.85%29.aspx 

which might provide connected state. Also there's the WM_INPUT_DEVICE_CHANGE event that fires when devices connect/disconnect. This might pick up on bluetooth/usb devices connecting/disconnecting.

Comment 5

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

I spent some time looking at the code around ::CM_Get_Device_ID in WinIMEHandler.cpp today to see if there was a way to determine if the keyboard was connected via USB but I didn't see it. That may get us a small bit of the way there, but I agree that we really need the event source information at the IME level to get this to work correctly.

Comment 6

[:Gijs (he/him)]

(In reply to Jim Mathies [:jimm] from comment #4)
> I think we would like to have both to do this right. I'd bet we can find a
> way to update IMEStateManager::NotifyIME to pass event source information
> through, it's available via gecko/dom events. If we don't want to tackle
> that here, lets get a bug filed on it.

Let's tackle that here. AFAICT these get created here:

https://dxr.mozilla.org/mozilla-central/rev/e2a910c048dc82fc3be53475f18e7f81f03e377b/dom/events/IMEStateManager.cpp#1311-1327

from here:

https://dxr.mozilla.org/mozilla-central/rev/e2a910c048dc82fc3be53475f18e7f81f03e377b/dom/events/IMEContentObserver.cpp#1472

from here:

https://dxr.mozilla.org/mozilla-central/rev/e2a910c048dc82fc3be53475f18e7f81f03e377b/dom/events/IMEContentObserver.cpp#1108

from here:

https://dxr.mozilla.org/mozilla-central/rev/e2a910c048dc82fc3be53475f18e7f81f03e377b/dom/events/IMEContentObserver.cpp#1141-1149

from here: 

https://dxr.mozilla.org/mozilla-central/rev/e2a910c048dc82fc3be53475f18e7f81f03e377b/dom/events/IMEContentObserver.cpp#254

which is annoying because at that point we're basically at the Init() and reinitialize calls of the IMEContentObserver, which are still, I imagine, pretty far away from the initial event that triggers the focus here. :-(

Masayuki, can you help? If we need the event that triggered the focus here, where would we get it, and how do we pass this information down without cluttering up all these call paths?

Comment 7

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

What's the "event source information"? Where is it?

Comment 8

[:Gijs (he/him)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #7)
> What's the "event source information"?

I'd like to know what type of event triggered the IME's focus. Was it a touch, mouse or keyboard event, was it JS/C++/the OS setting focus to the text entry item in question, etc.

> Where is it?

I was hoping you would be able to tell me that. :-)

Comment 9

[:Gijs (he/him)]

Hm, it looks like most calls will be through https://dxr.mozilla.org/mozilla-central/rev/e1ef2be156de1dad31bb4189a51b178b12b23340/editor/libeditor/nsEditorEventListener.cpp#1122-1123 , which is nsEditorEventListener's OnFocus handler. Which doesn't seem to have the source/"cause" of the focus event. :-(

Comment 10

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

How about to cache aAction.mCause in IMEHandler::SetInputContext()? When an editor gets focus, this is called first, then, NOTIFY_IME_OF_FOCUS is sent.

If you need to add other causes to InputContextAction::Cause, you need to hack nsFocusManager and some method callers of that.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Not sure if this is a regression from bug 1007063, or just continuing work, but tracking this for now. We could still take a batch in beta.

Comment 13

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r?enndeakin,jaws

Comment 14

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki

Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r?masayuki

Comment 15

[:Gijs (he/him)]

So these patches make things wfm.

The first patch adds a flag to nsIFocusManager and uses it in addition to FLAG_BYMOUSE when the mouse event that triggers focus was simulated because of a touch event.

This meant auditing all the uses of FLAG_BYMOUSE, which the patch does. In some cases I've only left comments; if you prefer I can take those out. For this bug I suspect the only change that's actually necessary is the one in the EventStateManager, but I figured we should be consistent. I looked at addons' MXR for people using BYMOUSE, and I don't think anybody will be negatively impacted (like the check in search.xml for which I flagged you, Jared, which I believe is just wrong either way). It is possible some add-ons would work better if they "opted in" to updating, but I've not looked into those cases too deeply.

The second part adds the IME handling portion, which isn't exposed through IDL and therefore shouldn't really break anything else.

What surprised me was that apart from bug 1224233 which threw me a bit, this seems to have worked as a first try, which I found very surprising and makes me think I've missed something. Do let me know if you have ideas about what I did miss. :-)

Comment 16

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

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

https://reviewboard.mozilla.org/r/25015/#review22559

r=me for the search.xml changes. Should we add a test for the search suggestions popup on click versus keyboard-initiated focus change?

Comment 17

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> Comment on attachment 8686661 [details]
> MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to
> nsIFocusManager, r?enndeakin,jaws
> 
> https://reviewboard.mozilla.org/r/25015/#review22559
> 
> r=me for the search.xml changes. Should we add a test for the search
> suggestions popup on click versus keyboard-initiated focus change?

Let's make one as part of bug 1222971. I don't think the change in this patch warrants creating one.

Comment 18

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

Comment on attachment 8686662 [details]
MozReview Request: Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki

https://reviewboard.mozilla.org/r/25017/#review22585

::: widget/windows/WinIMEHandler.cpp:42
(Diff revision 1)
> +InputContextAction::Cause IMEHandler::sLastContextActionCause = InputContextAction::CAUSE_UNKNOWN;

nit: Over 80 characters.

::: widget/windows/WinIMEHandler.cpp:353
(Diff revision 1)
> +  sLastContextActionCause = aAction.mCause;

I wonder, if you need to reopen VKB when user tap in editor (i.e., not actually focus changed), you need additional patch. See IMEStateManager::OnClickInEditor(). Although, this might be different bug, though.

::: widget/windows/WinIMEHandler.cpp:700
(Diff revision 1)
> +    Preferences::SetString(kOskDebugReason, L"IKPOS: Used touch to focus control, ignoring keyboard presence");

nit: over 80 characters.

Comment 19

[:Gijs (he/him)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #18)
> ::: widget/windows/WinIMEHandler.cpp:353
> (Diff revision 1)
> > +  sLastContextActionCause = aAction.mCause;
> 
> I wonder, if you need to reopen VKB when user tap in editor (i.e., not
> actually focus changed), you need additional patch. See
> IMEStateManager::OnClickInEditor(). Although, this might be different bug,
> though.

I'm not sure I understand. Can you expand on this? It does sound it's unrelated to this bug in that we've always been doing this when focus changed, and not at other times, and so if we want to add stuff for when focus doesn't change that should probably be a different bug?

Did you intend to not give r+, given that besides this you only noted nits? I know mozreview's UI is not always the most intuitive...

Comment 20

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

(In reply to :Gijs Kruitbosch from comment #19)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #18)
> > ::: widget/windows/WinIMEHandler.cpp:353
> > (Diff revision 1)
> > > +  sLastContextActionCause = aAction.mCause;
> > 
> > I wonder, if you need to reopen VKB when user tap in editor (i.e., not
> > actually focus changed), you need additional patch. See
> > IMEStateManager::OnClickInEditor(). Although, this might be different bug,
> > though.
> 
> I'm not sure I understand. Can you expand on this? It does sound it's
> unrelated to this bug in that we've always been doing this when focus
> changed, and not at other times, and so if we want to add stuff for when
> focus doesn't change that should probably be a different bug?
> 
> Did you intend to not give r+, given that besides this you only noted nits?
> I know mozreview's UI is not always the most intuitive...

Not a point of r-. I didn't open issue at here (unchecked the checkbox).

As I said, this should be another bug even if we need to fix so. See bug 507987 (when we developed Firefox for Maemo).

Comment 21

[:Gijs (he/him)]

Comment on attachment 8686662 [details]
MozReview Request: Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #20)
> Not a point of r-. I didn't open issue at here (unchecked the checkbox).

Ah, yes, but right now the review flag was just cleared, rather than setting r+. Per your comments I'm going to assume you did mean r+. :-)

Comment 22

[:Gijs (he/him)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Mike tells me Neil is on PTO, so redirecting the review for this portion to Olli.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Could you test if passing touch flag in FireFocusOnTargetContent would work well. That would be at least consistent.
FireFocusOnTargetContent is called by an event listener dealing with mouse events, so the information is easily available.
But you're probably right with the comment that when dealing with context menu not pretending that focus is from touch is better.
But please test.


For consistency RenderFrameParent::TakeFocusForClick() should use the right flag.
if I read the code correctly, there should be the touch flag always, but
I admit that setup would be a bit fragile given that the method itself doesn't know that it is for touch.
Could you ask kats or botond (https://bugzilla.mozilla.org/show_bug.cgi?id=1129526 added that code.) if it is safe to always pass
the touch flag too?

Comment 24

[:Gijs (he/him)]

(In reply to Olli Pettay [:smaug] from comment #23)
> Comment on attachment 8686661 [details]
> MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to
> nsIFocusManager, r?enndeakin,jaws
> 
> Could you test if passing touch flag in FireFocusOnTargetContent would work
> well. That would be at least consistent.
> FireFocusOnTargetContent is called by an event listener dealing with mouse
> events, so the information is easily available.
> But you're probably right with the comment that when dealing with context
> menu not pretending that focus is from touch is better.
> But please test.

I just realized that on Windows, which is where this is being implemented, this is immaterial because:

https://dxr.mozilla.org/mozilla-central/source/dom/xul/nsXULPopupListener.cpp#51

means that this code is all ifdef'd out there. I'll update the flags so the focus info is correct.

> For consistency RenderFrameParent::TakeFocusForClick() should use the right
> flag.
> if I read the code correctly, there should be the touch flag always, but
> I admit that setup would be a bit fragile given that the method itself
> doesn't know that it is for touch.
> Could you ask kats or botond
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1129526 added that code.) if
> it is safe to always pass
> the touch flag too?

Re-reading this code, I realized I didn't look far enough. You're right that this is all touch code anyway. I'll add a third patch and ask for review from kats or botond on that. Thanks!

Comment 25

[:Gijs (he/him)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

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

Comment 26

[:Gijs (he/him)]

Comment on attachment 8686662 [details]
MozReview Request: Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki

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

Comment 27

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1221947 - part 3: pass touch information to focus manager in RenderFrameParent, r?kats

Bug 1221947 - part 3: pass touch information to focus manager in RenderFrameParent, r?kats

Comment 28

[:Gijs (he/him)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

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

Comment 29

[:Gijs (he/him)]

Comment on attachment 8686662 [details]
MozReview Request: Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki

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

Comment 30

[:Gijs (he/him)]

Comment on attachment 8687923 [details]
MozReview Request: Bug 1221947 - part 3: pass touch information to focus manager in RenderFrameParent, r?kats

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

Comment 31

[:Gijs (he/him)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Gahhh, mozreview. :-\

Comment 32

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8687923 [details]
MozReview Request: Bug 1221947 - part 3: pass touch information to focus manager in RenderFrameParent, r?kats

https://reviewboard.mozilla.org/r/25263/#review22745

Comment 33

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e5bc99e6f928
https://hg.mozilla.org/integration/mozilla-inbound/rev/5af628d4e296
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa690942da0d

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e5bc99e6f928
https://hg.mozilla.org/mozilla-central/rev/5af628d4e296
https://hg.mozilla.org/mozilla-central/rev/aa690942da0d

Comment 35

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei, if we end up uplifting this fix to beta, can your team verify and do a bit of exploratory testing around this feature? We may want to do that for beta 5 or 6.

Comment 36

[:Gijs (he/him)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

NB: asking for approval for all 3 patches)

Approval Request Comment
[Feature/regressing bug #]: showing the on-screen keyboard on windows 8+
[User impact if declined]: on many Windows touch-screen devices, keyboards are present but can be folded back, in which case we should still show the on-screen keyboard if the user uses touch to interact with the screen. Without this patch, we will not do that, and using Firefox while such machines are in use as tablets would be very hard.
[Describe test coverage new/current, TreeHerder]: There is no test infrastructure for testing with touch machines, so sadly, no.
[Risks and why]: reasonably low considering:
- this feature has had significant QA attention already and this bug and dupes have been the main - almost the only - complaint (the only other one I know of is bug 1211234 which is Microsoft doing us a favour which our pref fails to turn off. We can live with that.).
- we have killswitch prefs both for the feature as a whole and for win8 specifically. On win10 the feature by default will only be usable within tablet mode (there is an OS-level pref to change this).
- I audited the code in mozilla-central as well as in our addons MXR and believe that the patch I landed fixed/addressed all the callsites/uses of the relevant focus manager code. We should update developer documentation when we're sure in which release we're shipping, but I don't really expect fallout - very few places interact with the focus manager so directly. Most code uses events and element.focus() .
[String/UUID change made/needed]: no. I added a constant to the nsIFocusManager.idl, which I believe does not require an IDL uuid rev, so I landed with IGNORE IDL. Uplifts may need to do the same.

Comment 37

[:Gijs (he/him)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Approval Request Comment: comment 36

(sorry, I meant to request both at once and forgot)

Comment 38

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Gijs, while reviewing the aurora uplift patch, I noticed that patch2 inside it has comments from masayuki on IME related aspects that still show up as "open issues" in MozReview tool. Do these need to be addressed? Or is this patch ready to land on m-a as is?

Comment 39

[:Gijs (he/him)]

(In reply to Ritu Kothari (:ritu) from comment #38)
> Gijs, while reviewing the aurora uplift patch, I noticed that patch2 inside
> it has comments from masayuki on IME related aspects that still show up as
> "open issues" in MozReview tool. Do these need to be addressed? Or is this
> patch ready to land on m-a as is?

The patch is ready to land. mozreview does not update issues addressed after review.

Comment 40

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to :Gijs Kruitbosch from comment #39)
> (In reply to Ritu Kothari (:ritu) from comment #38)
> > Gijs, while reviewing the aurora uplift patch, I noticed that patch2 inside
> > it has comments from masayuki on IME related aspects that still show up as
> > "open issues" in MozReview tool. Do these need to be addressed? Or is this
> > patch ready to land on m-a as is?
> 
> The patch is ready to land. mozreview does not update issues addressed after
> review.

Got it. Thanks!

Comment 41

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Improving our experience on windows touch-screen devices with keyboards folded back is a good idea! It seems QE has tested this area a bit and will be doing some focused testing when this is uplifted to Beta. Let's uplift to Aurora44.

Comment 42

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8686662 [details]
MozReview Request: Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki

Aurora44+

Comment 43

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8687923 [details]
MozReview Request: Bug 1221947 - part 3: pass touch information to focus manager in RenderFrameParent, r?kats

Aurora44+

Comment 44

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/8e13fe7ab3da
https://hg.mozilla.org/releases/mozilla-aurora/rev/54f8c04cc8d7
https://hg.mozilla.org/releases/mozilla-aurora/rev/36ad43168b8d

Comment 45

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8686661 [details]
MozReview Request: Bug 1221947 - part 1: add BYTOUCH flag to nsIFocusManager, r=jaws,smaug

Touch screen fix - Approved for uplift to beta - all 3 patches in this bug.

Comment 46

[Andrei Vaida [:avaida]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #35)
> Andrei, if we end up uplifting this fix to beta, can your team verify and do
> a bit of exploratory testing around this feature? We may want to do that for
> beta 5 or 6.

Sounds good to me, I'll make sure this gets looked at in Beta 5. Leaving the ni? in place as a reminder.

Comment 47

[Vasilica Tamas, QA [:vtamas]]

Performed Exploratory testing on latest Nightly and Aurora under Windows 10 64-bit using a Dell Xps 12 with tablet mode enabled and this is what I have noticed:
- This bug doesn't seem to be fixed on latest Nightly 45.0a1 (2015-11-17), the OSK does not appear when a text box is focused http://i.imgur.com/6fllICZ.png
- The OSK is successfully displayed on latest Aurora 44.0a2 (2015-11-17) while focusing a text box http://i.imgur.com/F0bdh1y.png


Noticed that the value of ui.osk.on_screen_keyboard_path is different on Nightly and Aurora. (compare the attached screenshots)


Also tested under a Dell Inspiron One All-In-One, with no keyboard attached and the OSK was not displayed: http://i.imgur.com/VZsTIw9.jpg.
Should I file a separate bug for Dell Inspiron One All-In-One issue?

Comment 48

[:Gijs (he/him)]

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #47)
> Performed Exploratory testing on latest Nightly and Aurora under Windows 10
> 64-bit using a Dell Xps 12 with tablet mode enabled and this is what I have
> noticed:
> - This bug doesn't seem to be fixed on latest Nightly 45.0a1 (2015-11-17),
> the OSK does not appear when a text box is focused
> http://i.imgur.com/6fllICZ.png
> - The OSK is successfully displayed on latest Aurora 44.0a2 (2015-11-17)
> while focusing a text box http://i.imgur.com/F0bdh1y.png
> Noticed that the value of ui.osk.on_screen_keyboard_path is different on
> Nightly and Aurora. (compare the attached screenshots)

This doesn't make any sense. Both of those debug reason screenshots indicate the keyboard *wasn't* displayed because we found a hardware keyboard. The code in Aurora and Nightly should be the same at this point. However, your build ID is for aurora from yesterday, which by the timestamp in comment #44 wouldn't even have these patches - they landed evening GMT, by which time the build for aurora would have been long finished. You should be using today's build for tests. I'm not even sure yesterday's nightly build had the patches - an 11am GMT nightly merge (comment 34) doesn't usually make that day's nightly.

How are you focusing the textbox? What textbox are you using to test with? (see also the password input bug 1211234 which is relevant here). Note also that, for instance, the search bar has code for clicks close to it to focus it manually - that doesn't propagate any of the focus reasons. We should fix that separately, but for testing this bug you should really just use an in-content HTML page with a plain input field for consistent results.

> Also tested under a Dell Inspiron One All-In-One, with no keyboard attached
> and the OSK was not displayed: http://i.imgur.com/VZsTIw9.jpg.
> Should I file a separate bug for Dell Inspiron One All-In-One issue?

Yes, but that's less relevant here. I'm really more interested in the XPS 12 issue right now.

Comment 49

[:Gijs (he/him)]

I have now verified that the 11-17 nightly indeed didn't have these patches. So please retest with today's nightly and aurora builds.

Comment 50

[Carsten Book [:Tomcat]]

part 2 has problems to be uplifted to beta:

grafting 315339:54f8c04cc8d7 "Bug 1221947 - part 2: add TOUCH cause to IME Handling and use it to avoid checking for a keyboard, r=masayuki a=ritu"
merging dom/base/nsFocusManager.cpp
merging widget/IMEData.h
merging widget/windows/WinIMEHandler.cpp
warning: conflicts during merge.
merging widget/windows/WinIMEHandler.cpp incomplete! (edit conflicts, then use 'hg resolve --mark')
merging widget/windows/WinIMEHandler.h
warning: conflicts during merge.
merging widget/windows/WinIMEHandler.h incomplete! (edit conflicts, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)

can you take a look, thanks!

Comment 51

[Vasilica Tamas, QA [:vtamas]]

(In reply to :Gijs Kruitbosch from comment #48)

> This doesn't make any sense. Both of those debug reason screenshots indicate
> the keyboard *wasn't* displayed because we found a hardware keyboard. The
> code in Aurora and Nightly should be the same at this point. However, your
> build ID is for aurora from yesterday, which by the timestamp in comment #44
> wouldn't even have these patches - they landed evening GMT, by which time
> the build for aurora would have been long finished. You should be using
> today's build for tests. I'm not even sure yesterday's nightly build had the
> patches - an 11am GMT nightly merge (comment 34) doesn't usually make that
> day's nightly.

Sorry, my mistake, checked again and noticed that was Aurora from 2015-11-18.
Tested today on Nightly 2015-11-18 and the osk is successfully displayed. 


> How are you focusing the textbox? What textbox are you using to test with?
> (see also the password input bug 1211234 which is relevant here). Note also
> that, for instance, the search bar has code for clicks close to it to focus
> it manually - that doesn't propagate any of the focus reasons. We should fix
> that separately, but for testing this bug you should really just use an
> in-content HTML page with a plain input field for consistent results.

I test around the search bars from: location bar, search bar, about:newtab page, about:home page and text boxes from different webpages or http://www.quackit.com/html/codes/html_text_box_code.cfm 


Also encountered 2 new issues: Bug 1226145 and Bug 1226148

Comment 52

[:Gijs (he/him)]

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #51)

Thanks for the quick verification. I'm glad it works now - you had me panicked for a few minutes yesterday! :-)

> Also encountered 2 new issues: Bug 1226145 and Bug 1226148

Thanks. I'll look at these either tomorrow or next week. They're not regressions and should be covered by bug 1224605, I believe.

Did you also file the issue with the Dell Inspiron One All-in-One?

(In reply to Carsten Book [:Tomcat] from comment #50)
> part 2 has problems to be uplifted to beta:
> 
> grafting 315339:54f8c04cc8d7 "Bug 1221947 - part 2: add TOUCH cause to IME
> Handling and use it to avoid checking for a keyboard, r=masayuki a=ritu"
> merging dom/base/nsFocusManager.cpp
> merging widget/IMEData.h
> merging widget/windows/WinIMEHandler.cpp
> warning: conflicts during merge.
> merging widget/windows/WinIMEHandler.cpp incomplete! (edit conflicts, then
> use 'hg resolve --mark')
> merging widget/windows/WinIMEHandler.h
> warning: conflicts during merge.
> merging widget/windows/WinIMEHandler.h incomplete! (edit conflicts, then use
> 'hg resolve --mark')
> abort: unresolved conflicts, can't continue
> (use hg resolve and hg graft --continue)
> 
> can you take a look, thanks!

I'll have a look ASAP - I got sidetracked with bug 1226136 earlier today.

Comment 53

[:Gijs (he/him)]

Landed on beta:

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/02acffd9874e
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/73fdfb5d27e7
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/7c2204493788

Comment 54

[Vasilica Tamas, QA [:vtamas]]

(In reply to :Gijs Kruitbosch from comment #52)

> Did you also file the issue with the Dell Inspiron One All-in-One?

Filed Bug 1226522

Comment 56

[Vasilica Tamas, QA [:vtamas]]

Verified fixed on Firefox 45.0a1 (2015-12-02), Firefox 44.0a2 (2015-12-02) and Firefox 43 beta 8 (20151201152349) under Windows 8.1 64-bit desktop mode and Windows 10 64-bit tablet mode on/off.


We also performed a regression testing on Firefox 43 beta 6 under Windows 8.1 and Windows 10 using 2 devices: Dell Xps 12 and Surface Pro 2. Except a few already known bugs, there were some new issues encountered during this testing. 
    - Bugs that are still affecting the feature: https://goo.gl/PHoVHO 
    - Detailed test results: https://goo.gl/t4VPS3


Based on the above mentioned, I am marking this bug as Verified since the other issues are tracked separately.

Comment 57

[Alex Taylor]

Apologies in advance if location or format of my query is wrong!

Hi -I've landed here as 43 is now popping the keyboard unexpectedly for me in Windows 10.
My use case where the problem arises is rare but probably not unique:  I am using a Surface Pro 3 without a physical keyboard attached, but in desktop mode with keyboard input via Synergy (http://synergy-project.org/).  In this situation I am using a physical keyboard, but the OSK keeps popping up.
In searching for info I see other users with bluetooth keyboard anomalies (https://support.mozilla.org/questions/1103572).

Is it possible for an option make the OSK behaviour (which I really like in Tablet Mode), be dependent on Windows Tablet Mode?

Comment 58

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

(In reply to Alex Taylor from comment #57)
> Apologies in advance if location or format of my query is wrong!
> 
> Hi -I've landed here as 43 is now popping the keyboard unexpectedly for me
> in Windows 10.
> My use case where the problem arises is rare but probably not unique:  I am
> using a Surface Pro 3 without a physical keyboard attached, but in desktop
> mode with keyboard input via Synergy (http://synergy-project.org/).  In this
> situation I am using a physical keyboard, but the OSK keeps popping up.
> In searching for info I see other users with bluetooth keyboard anomalies
> (https://support.mozilla.org/questions/1103572).
> 
> Is it possible for an option make the OSK behaviour (which I really like in
> Tablet Mode), be dependent on Windows Tablet Mode?

We should only be showing the OSK outside of Tablet mode if you've enabled that behavior in Windows 10. Go to the Windows Settings dialog > Devices > Typing > "Show the touch keyboard or handwriting panel when not in tablet mode and there's no keyboard attached". Set that pref to Off.

Comment 59

[Alex Taylor]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #58)
> We should only be showing the OSK outside of Tablet mode if you've enabled
> that behavior in Windows 10. Go to the Windows Settings dialog > Devices >
> Typing > "Show the touch keyboard or handwriting panel when not in tablet
> mode and there's no keyboard attached". Set that pref to Off.
Fantastic - worked perfectly.  I may post the solution over in the support area.
Thanks for the speedy reply, and keep up the great work.

Comment 60

[Unreal]

Hi,
I have a Surface Pro 8. Every other program including Edge shows the on-screen keyboard when my keyboard is not plugged and it is needed. Firefox consistently never pop ups the keyboard, not even in the URL bar making it almost unusable.

Specs:
Surface Pro 8
Windows 10 Home 21H2 (last update) 64bit
Firefox 97.0 (64-bit)

"Show the touch keyboard or handwriting panel when not in tablet mode and there's no keyboard attached" is set to ON.

Thank you for your help.

PS. I wish we could paste images here to help you debug those problems. I also found that the strong password criteria was exaggerated for a user that simply wants to create a quick account to report a bug. It took me 2min to find a suitable password which I will forget because it is too complicated.

Comment 61

[Unreal]

Here's what appears in about:config (osk settings) when my keyboard is unplugged:
https://user.fm/files/v2-489a85c514f12a1fa9ebdc5aed4931e7/osk.png

This happens if I unplug the keyboard while I run Firefox, or even if I close Firefox, unplug the keyboard and open it again.

Comment 62

[Unreal]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:97.0) Gecko/20100101 Firefox/97.0

Steps to reproduce:

On a Surface Pro 8, Windows 10 Home 21H2 (last update) 64bit

Unplug keyboard
Click on URL bar

Actual results:

On-screen keyboard doesn't pop up

Expected results:

On-screen keyboard should pop up like it does for Notepad++ or Microsoft Edge.

Comment 63

[:Gijs (he/him)]

(In reply to Unreal from comment #60)

Hi,
I have a Surface Pro 8. Every other program including Edge shows the on-screen keyboard when my keyboard is not plugged and it is needed. Firefox consistently never pop ups the keyboard, not even in the URL bar making it almost unusable.

I filed bug 1754973 for you. It's best to track issues like this in a new bug, rather than in a closed bug from 6 years ago (when the SP8 did not exist!)