480306 - Cannot enter password on gmail login using the soft keyboard

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

I am trying to log into gmail using the soft keyboard.  I enter my username just fine, but when I click on the password box (and it turns yellow and the soft keyboard pops up), I tap and no * shows up in the box.

If I slide the screen up and type on the keyboard I can get * chars in the password box.

Comment 1

[Rob Russell]

I am also getting this bug, in M10 and M12, Mozilla Fennec 1.0b1, on my Nokia n800.

I found the following duplicates:
https://bugzilla.mozilla.org/show_bug.cgi?id=483964
https://bugzilla.mozilla.org/show_bug.cgi?id=482739

Comment 3

[Doug Turner (:dougt)]

Attachment: patch v.1

untested.  build here: http://www.meer.net/users/dougt/fennec-1.0b1.en-US.linux-arm.tar.bz2

Comment 4

[Antonio Gomes (tonikitoo)]

dougt, good catch. It was probably my fault ... =/

Comment 5

[Doug Turner (:dougt)]

no biggie, the same thing was in the wince code.  do you have a n800 to verify?

Comment 6

[Doug Turner (:dougt)]

just tested on the n810 and worked fine.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

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

I have the same question on this as I did with the wince fix, why isn't the IME enabled for passwords in general?

Antonio, for the record, you're off the hook, this is my code (just checked hg blame)

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

I tried this build:
http://www.meer.net/users/dougt/fennec-1.0b1.en-US.linux-arm.tar.bz2

and was unable to enter a password into a password field (like gmail).

We also need to make sure that the autosuggest words that are on the N810 do not use the password.  for exammple I entered my username and password, then was typing something similar to my password in a text field and the nokia suggested my password.

Comment 9

[Antonio Gomes (tonikitoo)]

> We also need to make sure that the autosuggest words that are on the N810 do
> not use the password.  for exammple I entered my username and password, then
> was typing something similar to my password in a text field and the nokia
> suggested my password.

joel, that is what this piece of code it about (should be)

http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsWindow.cpp#6621

Comment 10

[Doug Turner (:dougt)]

yeah, there is something wrong with the software keyboard input in general.  Another problem i am seeing -- if you type something, then dismiss the software keyboard, then want to edit what you previous typed, you can't.

blassey, what about this:
http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsWindow.cpp#6630

Do we really want this or'd in?  My reading of the header seems to suggest that after you enter text into a SIP, and this flag is set, that we will drop whatever was entered.

Comment 11

[Doug Turner (:dougt)]

i filed 484408 for the re-editing problem.

Comment 12

[Doug Turner (:dougt)]

tonikitoo is taking a look over the weekend.

Comment 13

[Antonio Gomes (tonikitoo)]

so, I got into this proble just this weekend. It is failing at this check (in the IM_commit_ routine):


void
IM_commit_cb(GtkIMContext *aContext,
             const gchar  *aUtf8_str,
             nsWindow     *aWindow)

(...)
    if (!window || IM_get_input_context(window) != aContext)
        return;

a |wrong| context seems to be returning.

investigating ...

Comment 14

[Antonio Gomes (tonikitoo)]

I wend deeper into it, yesterday and this is what is happening in this bug:

* when user taps an input field, nsWindow::SetIMEEnabled(PRUint32 aState) is called. The incoming parameter |aState| can be 
- IME_STATUS_DISABLED = 0
- IME_STATUS_ENABLED = 1
- IME_STATUS_PASSWORD = 2.

In the case of a PASSWD field, the later in passed in as expected. However @ SetIMEEnabled , |hildon_gtk_im_context_show| method (what forces the vbk to show it up) gets *ALWAYS* the the mContext as parameter AND it is not the context for passwd fields, but mSimpleContext, according to the header:

// Actual context. This is used for handling the user's input.
GtkIMContext       *mContext;
// mSimpleContext is used for the password field and
// the |ime-mode: disabled;| editors. These editors disable IME.
// But dead keys should work. Fortunately, the simple IM context of
// GTK2 support only them.
GtkIMContext       *mSimpleContext;
// mDummyContext is a dummy context and will be used in IMESetFocus()
// when mEnabled is false. This mDummyContext IM state is always
// "off", so it works to switch conversion mode to OFF on IM status
// window.


See last line of snippet below:

(... SetIMEEnabled ...)
    GtkIMContext *focusedIm = nsnull;
    // XXX Don't we need to check gFocusWindow?
    nsRefPtr<nsWindow> focusedWin = gIMEFocusWindow;
    if (focusedWin && focusedWin->mIMEData)
        focusedIm = focusedWin->mIMEData->mContext;
(...)

that should be something like this:

(... SetIMEEnabled ...)
    GtkIMContext *focusedIm = nsnull;
    // XXX Don't we need to check gFocusWindow?
    nsRefPtr<nsWindow> focusedWin = gIMEFocusWindow;
    if (focusedWin && focusedWin->mIMEData)
        if (aState == IME_STATUS_ENABLED)
            focusedIm = mIMEData->mContext;
        else if (aState == nsIWidget::IME_STATUS_PASSWORD)
            focusedIm = mIMEData->mSimpleContext;
(...)

So in the case of passwd, it is calling 

hildon_gtk_im_context_show(mContext);

BUT the callback fails because it was expecting a call like 

hildon_gtk_im_context_show(mSimpleContext);



but why all this ? because in the "commit" callback (called when something is going to be shown by the vkb) checks if the context's match. this used to work before, but not now due to the checkin in bug 472635.

Comment 15

[Antonio Gomes (tonikitoo)]

brad, thoughts about comment #14 above ?

Comment 16

[Antonio Gomes (tonikitoo)]

> but why all this ? because in the "commit" callback (called when something is
> going to be shown by the vkb) checks if the context's match. this used to work
> before, but not now due to the checkin in bug 472635.

to be more precise:

IM_commit_cb was not check'ing if the right aContext was being passed in. But after 

http://hg.mozilla.org/mozilla-central/rev/9860a796f20d

it is. So as we call hildon_gtk_context_show w/ mContext as parameter and commit_cb passes in mSimpleContext, it bails out at the check.

Comment 17

[Antonio Gomes (tonikitoo)]

Attachment: v0.0 - rought patch that makes it work again ...

Comment 18

[Antonio Gomes (tonikitoo)]

Attachment: v0.1 - rought patch that makes it work again ...

err, right patch 0.1.

sorry for spamming.

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 371239 [details] [diff] [review]
v0.1 - rought patch that makes it work again ...

Ginn, can you take a look at this change to make sure we don't break what you did in bug 472635. See comment 14.

Comment 20

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 371239 [details] [diff] [review]
v0.1 - rought patch that makes it work again ...

looks good.  For the record though, I'm not a peer.

Comment 21

[Antonio Gomes (tonikitoo)]

Comment on attachment 371239 [details] [diff] [review]
v0.1 - rought patch that makes it work again ...

not sure if gen is peer too, so requesting r+ from stuart ...

ps: should I add a MOZ_HILDON_PLATFORM ifdef to the solution proposed in the patch ?

Comment 22

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #21)

> ps: should I add a MOZ_HILDON_PLATFORM ifdef to the solution proposed in the
> patch ?

Not unless we think it could break other builds. We should try to avoid ifdefs when we can.

Comment 23

[Antonio Gomes (tonikitoo)]

pushed. 8c64012db5ee

it should also land on 1.9.1 but I presume it has to be blocking-fennec1.0+ for that.

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 371239 [details] [diff] [review]
v0.1 - rought patch that makes it work again ...

a191=beltzner

Comment 25

[Joe Drew (not getting mail)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/609f08083ec7

Comment 26

[Joel Maher ( :jmaher ) (UTC -8)]

verified with maemo nightly 20090818