511534 - Poor interaction with the software keyboard

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Doug Turner (:dougt)]

When the software keyboard is enabled on the n810 (or on the n800), the software keyboard commonly does not interact as expected in Fenenc.

For example, if you click on the URL bar, you will see the software keyboard appear.  However, dismissing the software keyboard does not happen even if you press the "return" software key or the the "dismiss" software key.

Pressing the "return" key correctly puts the entered text in the text area of the browser, but the software keyboard is displayed again because focus remains in the text area. 

Pressing the "dismiss" key correctly dismisses the keyboard, but it is displayed again because focus remains in the text area.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

This seems to fix the problem.

Comment 2

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

Comment on attachment 396777 [details] [diff] [review]
patch v.1

I'm a owner of nsIMEStateManager, but I cannot review nsFocusManager, so, Enn should review this too.

Enn:

nsIMEStateManager::OnChangeFocus shouldn't be called at deactivating all our windows. I.e., when non-our window gets the focus, we shouldn't change the IME state of the last focused window.

Before the focus refactoring, the method did nothing when we are deactive. But the method cannot know whether we are active or not now. Therefore, we shouldn't call the method at that time.

I couldn't find this meaning change at the focus refactoring, sorry.

And this bug should be major bug on some Linux environments which use some Window Managers which moves the focus to a popuped panel. On such environments, IME cannot use correctly because when IMEs open a popup panel (e.g., a candidate window or a reading window), we are deactivated by the panel gets focus. Then, we commit the current composition. Of course, it shouldn't be so.

Comment 3

[Neil Deakin]

(In reply to comment #2)
> nsIMEStateManager::OnChangeFocus shouldn't be called at deactivating all our
> windows. I.e., when non-our window gets the focus, we shouldn't change the IME
> state of the last focused window.

Can you list the situations when OnChangeFocus should not be called? Is it only when Blur() is called from WindowLowered()? If so, comparing mActiveWindow to null is the way to do that.

Comment 4

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

(In reply to comment #3)
> (In reply to comment #2)
> > nsIMEStateManager::OnChangeFocus shouldn't be called at deactivating all our
> > windows. I.e., when non-our window gets the focus, we shouldn't change the IME
> > state of the last focused window.
> 
> Can you list the situations when OnChangeFocus should not be called? Is it only
> when Blur() is called from WindowLowered()? If so, comparing mActiveWindow to
> null is the way to do that.

I cannot list up the negative situations because it wanted to be called only when following situations:

* when the focus is unset from the focused element when we are active.
* when a element gets focus.

So, in other words, it shouldn't be called when the active element in focused window is not changed.

Comment 5

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

(In reply to comment #4)
> I cannot list up the negative situations because it wanted to be called only
> when following situations:
> 
> * when the focus is unset from the focused element when we are active.
> * when a element gets focus.

Ah, sorry. This is correct:

* when the focus is unset in active window.
* when an element gets focus in active window.
* when a window is activated.

Comment 6

[Neil Deakin]

(In reply to comment #5)
> * when the focus is unset in active window.

So it sounds like you just want to check mActiveWindow again.

For instance, the two arguments to Blur() could both be null when switching from one subframe to another as well.

Comment 7

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

(In reply to comment #6)
> (In reply to comment #5)
> > * when the focus is unset in active window.
> 
> So it sounds like you just want to check mActiveWindow again.

Sounds ok for me, I checked the nsFocusManager briefly.

Doug:

Would you create a new patch? I'll sleep soon, unfortunately. You can carry over the my r+ if you only change the if statement to |if (mActiveWindow)|.

Thank you.

Comment 8

[Doug Turner (:dougt)]

Attachment: patch v.2

what Masayuki suggested.

Comment 9

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/62fb8747c8a9

Comment 10

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6596b98229af

Comment 11

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

Thank you, Doug!