On-screen keyboard should stay open (rather than hiding and reshowing or just plain hiding) when switching focus between two inputs/textareas/contenteditables

VERIFIED FIXED in Firefox 45

Status

()

Core
Widget: Win32
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Vasilica Mihasca, QA [away for an extended period of time - please needinfo addonsqa@softvision.ro], Assigned: Gijs)

Tracking

(Depends on: 1 bug)

Trunk
mozilla47
x86_64
Windows 10
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox44 affected, firefox45 verified, firefox46 verified, firefox47 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Note: this is a follow-up for Bug 1221947

Reproducible on: Firefox 45.0a1 and Firefox 44.0a2 using Windows 

Prerequisites: Tabled mode enabled

STR
1.Launch Firefox with a clean profile.
2.Navigate to https://login.yahoo.com/
3.Type your username using OSK.
4.Click Tab key from OSK in order to focus the password field.

ER
The OSK remains displayed.

AR
The OSK closes.
- This issue is also reproducible while changing the focus from url bar to search bar using the OSK Tab key.


Additional notes:
- This issue is reproducible on Firefox 45.0a1 (2015-11-18) and Firefox 44.0a2 (2015-11-18) under Windows 10 64-bit.
- This testing was performed on a Dell Xps 12 with tabled mode enabled.

Comment 1

2 years ago
I see the same happening on Win 8.1, Firefox 43.0.2.
(Assignee)

Comment 2

2 years ago
I expect that this will need to be fixed in the same way as bug 1227930, which I think means we need the relatedTarget for the focus and blur events that we're using in IME. Unfortunately we don't implement relatedTarget for focus and blur events (bug 962251). So this is not easy to solve. Masayuki, am I right? Is there an easier solution?
Flags: needinfo?(masayuki)
(In reply to :Gijs Kruitbosch from comment #2)
> I expect that this will need to be fixed in the same way as bug 1227930,
> which I think means we need the relatedTarget for the focus and blur events
> that we're using in IME. Unfortunately we don't implement relatedTarget for
> focus and blur events (bug 962251). So this is not easy to solve. Masayuki,
> am I right? Is there an easier solution?

I wonder, cannot we put off to close OSK with ::PostMessageW() or something? When new focused element causes opening OSK, you can remove the message from the queue. We are already using similar hack with WM_APP + n messages.
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindowDefs.h?rev=9104abe421a8&mark=23-41#23
Flags: needinfo?(masayuki)
(Assignee)

Comment 4

2 years ago
Created attachment 8708991 [details] [diff] [review]
WIP patch

Is this more or less what you had in mind? I noticed that while it mostly works, it doesn't work when focus moves between the parent and child process, sometimes leading to the OSK being hidden when it should be shown, and sometimes leading 'just' to flickering. Do you have an idea how to fix that? I'm assuming I should be passing a message somehow, but I'm worried that that is going to race with the PostMessage call on the window...
Attachment #8708991 - Flags: feedback?(masayuki)
(Assignee)

Updated

2 years ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8708991 [details] [diff] [review]
WIP patch

I think that you don't need to add sOtherFieldGotFocusSinceBlur since you can check sFocusedWindow (it's set at NOTIFY_IME_OF_FOCUS and cleared at NOTIFY_IME_OF_BLUR).

And I think that you don't need to create nsWindow::PostOSKDismissalMsg(). You should call ::PostMessageW() directly from IMEHandler::MaybeDismissOnScreenKeyboard().
Attachment #8708991 - Flags: feedback?(masayuki) → feedback+
(Assignee)

Comment 6

2 years ago
Thanks for the feedback!

(In reply to :Gijs Kruitbosch from comment #4)
> I noticed that while it mostly
> works, it doesn't work when focus moves between the parent and child
> process, sometimes leading to the OSK being hidden when it should be shown,
> and sometimes leading 'just' to flickering. Do you have an idea how to fix
> that? I'm assuming I should be passing a message somehow, but I'm worried
> that that is going to race with the PostMessage call on the window...

I would still really like to know what you think about this issue. :-)
Flags: needinfo?(masayuki)
(In reply to :Gijs Kruitbosch from comment #6)
> Thanks for the feedback!
> 
> (In reply to :Gijs Kruitbosch from comment #4)
> > I noticed that while it mostly
> > works, it doesn't work when focus moves between the parent and child
> > process, sometimes leading to the OSK being hidden when it should be shown,
> > and sometimes leading 'just' to flickering. Do you have an idea how to fix
> > that? I'm assuming I should be passing a message somehow, but I'm worried
> > that that is going to race with the PostMessage call on the window...
> 
> I would still really like to know what you think about this issue. :-)

When focus moves from content to chrome, the notification order may be:

NOTITY_IME_OF_FOCUS (chrome) -> NOTIFY_IME_OF_BLUR (content)

However, the delayed NOTIFY_IME_OF_BLUR is discarded in IMEStateManager::NotifyIME():
http://mxr.mozilla.org/mozilla-central/source/dom/events/IMEStateManager.cpp?rev=d83462d4d148&mark=1382-1386#1360

So, the delayed NOTIFY_IME_OF_BLUR won't be reached to IMEHandler.

On the other hand, focus moves from chrome to content, the order of notifications must be normal.

So, I have no idea why that occurs.
Flags: needinfo?(masayuki)
(Assignee)

Comment 8

2 years ago
Created attachment 8712326 [details] [diff] [review]
reduce flickering and closing when using the OSK tab key,

Note that I added sFocusedWindow setting/resetting to the non-TSF-enabled case in the message handler, because AFAICT we expect to use it irrespective of whether TSF is enabled/disabled. Let me know if that's wrong and I should remove it (but, in that case, will the code here work as expected?). I could no longer reproduce the flickering/issues with e10s, so it seems that this patch is otherwise good to go. :-)
Attachment #8712326 - Flags: review?(masayuki)
(Assignee)

Updated

2 years ago
Attachment #8708991 - Attachment is obsolete: true
Comment on attachment 8712326 [details] [diff] [review]
reduce flickering and closing when using the OSK tab key,

> // static
> void
>-IMEHandler::MaybeDismissOnScreenKeyboard()
>+IMEHandler::MaybeDismissOnScreenKeyboard(nsWindow* aWindow)
> {
>   if (sPluginHasFocus ||
>       !IsWin8OrLater()) {
>     return;
>   }
> 
>-  IMEHandler::DismissOnScreenKeyboard();
>+  ::PostMessage(aWindow->GetWindowHandle(), MOZ_WM_DISMISS_ONSCREEN_KEYBOARD, 0, 0);

nit: too long line, please wrap after |MOZ_WM_DISMISS_ONSCREEN_KEYBOARD,|.
Attachment #8712326 - Flags: review?(masayuki) → review+
(Assignee)

Updated

2 years ago
Blocks: 1243345

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6c9406cbc501
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1227930
(Assignee)

Comment 13

2 years ago
I would like to uplift this to 45 (see the open blocking bug) but I would also like to be more sure that this fix didn't regress anything important before I request uplift. Vasilica, what's the process to get this fix verified and/or to do a little exploratory testing to see if it regressed something?
Flags: needinfo?(vasilica.mihasca)
(Assignee)

Updated

2 years ago
Summary: On-screen keyboard closes while focusing a text field using OSK Tab key → On-screen keyboard should stay open (rather than hiding and reshowing or just plain hiding) when switching focus between two inputs/textareas/contenteditables
Performed Exploratory testing around this bug and I noticed the following:

- Intermittently, the osk is not displayed for yahoo password field while switching the focus using the Tab key (or the osk is hiding and reshowing if I touch the field ) - a browser restart solves the problem. This reproduces with clean profile and I did not managed to establish clear steps. I will continue to investigate this issue.

- Encountered 2 new issues:
[1] Bug 1245515 - Short flicker with the Xul page link on the OSK (regressed by this bug)
[2] Bug 1245522 - Password protection OSK button is not automatically displayed while focusing a password field

Tested on Firefox 47.0a1 (2016-02-02) under Windows 10 64-bit using Dell Xps 12 and Surface Pro 2.
Flags: needinfo?(vasilica.mihasca)
(Assignee)

Comment 15

2 years ago
Comment on attachment 8712326 [details] [diff] [review]
reduce flickering and closing when using the OSK tab key,

Approval Request Comment
[Feature/regressing bug #]: Windows on-screen keyboard support
[User impact if declined]: the on-screen keyboard jumps about when switching between fields, rather than remaining visible, making it irritating and confusing to work with
[Describe test coverage new/current, TreeHerder]: no, but extra QA was done
[Risks and why]: low, because of the extra QA here
[String/UUID change made/needed]: no
Attachment #8712326 - Flags: approval-mozilla-beta?
Attachment #8712326 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

2 years ago
Depends on: 1227925
Comment on attachment 8712326 [details] [diff] [review]
reduce flickering and closing when using the OSK tab key,

Improve the windows 10 support, taking it.
Should be in 45 beta 5
Attachment #8712326 - Flags: approval-mozilla-beta?
Attachment #8712326 - Flags: approval-mozilla-beta+
Attachment #8712326 - Flags: approval-mozilla-aurora?
Attachment #8712326 - Flags: approval-mozilla-aurora+

Comment 17

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/4b9d05addfe5
status-firefox46: --- → fixed
has problems uplifting to beta:

Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r 4b9d05addfe5
grafting 327687:4b9d05addfe5 "Bug 1226148 - reduce flickering and closing when using the OSK tab key, r=masayuki, a=sylvestre"
merging widget/windows/WinIMEHandler.cpp
merging widget/windows/WinIMEHandler.h
merging widget/windows/nsWindowDefs.h
warning: conflicts while merging widget/windows/nsWindowDefs.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 19

2 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/8b5409c04676
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #19)
> https://hg.mozilla.org/releases/mozilla-beta/rev/8b5409c04676ss

setting flags
status-firefox45: affected → fixed
As mentioned in Comment 14 this issue still intermittently reproduces on Firefox 47.0a1 (2016-02-21) and Firefox 46.0a2 (2016-02-22) with a clean profile under Dell Xps 12. (it is not reproducible under Surface Pro 2).A browser restart solves the problem.

It seems to be fixed on Firefox 45 beta 8 (20160218171844).

Gijs, how we should proceed in this situation? Should I reopen this bug or file a new one?
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 22

2 years ago
(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #21)
> As mentioned in Comment 14 this issue still intermittently reproduces on
> Firefox 47.0a1 (2016-02-21) and Firefox 46.0a2 (2016-02-22) with a clean
> profile under Dell Xps 12. (it is not reproducible under Surface Pro 2).A
> browser restart solves the problem.
> 
> It seems to be fixed on Firefox 45 beta 8 (20160218171844).
> 
> Gijs, how we should proceed in this situation? Should I reopen this bug or
> file a new one?

Please file a new bug with more details about how the xps 12 differs from the sp2, what OS they are running etc.

But it's fixed on the xps 12 as well on 45, but not 46? That is highly surprising...
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #22)

> Please file a new bug with more details about how the xps 12 differs from
> the sp2, what OS they are running etc.

Filed Bug 1250484

Based on Comment 21 and Comment 22, I am marking this bug as Verified since the other issue is tracked separately.
Status: RESOLVED → VERIFIED
status-firefox45: fixed → verified
status-firefox46: fixed → verified
status-firefox47: fixed → verified
You need to log in before you can comment on or make changes to this bug.