Closed Bug 480306 Opened 11 years ago Closed 10 years ago

Cannot enter password on gmail login using the soft keyboard

Categories

(Core :: Widget: Gtk, defect)

ARM
Maemo
defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: jmaher, Assigned: tonikitoo)

References

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 2 obsolete files)

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.
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
Duplicate of this bug: 483964
Assignee: nobody → doug.turner
Assignee: doug.turner → nobody
Assignee: nobody → doug.turner
dougt, good catch. It was probably my fault ... =/
no biggie, the same thing was in the wince code.  do you have a n800 to verify?
just tested on the n810 and worked fine.
Attachment #368464 - Flags: superreview?(pavlov)
Attachment #368464 - Flags: review?(bugmail)
Attachment #368464 - Flags: review?(bugmail) → review+
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)
Attachment #368464 - Flags: superreview?(pavlov) → superreview+
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.
> 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
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.
i filed 484408 for the re-editing problem.
tonikitoo is taking a look over the weekend.
Assignee: doug.turner → 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 ...
Status: NEW → ASSIGNED
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.
brad, thoughts about comment #14 above ?
> 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.
Attachment #368464 - Attachment is obsolete: true
err, right patch 0.1.

sorry for spamming.
Attachment #371237 - Attachment is obsolete: true
Attachment #371239 - Flags: review?(bugmail)
Attachment #371239 - Flags: review?(ginn.chen)
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.
Attachment #371239 - Flags: review?(ginn.chen) → review+
tracking-fennec: --- → ?
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.
Attachment #371239 - Flags: review?(bugmail) → review+
Attachment #371239 - Flags: superreview?(pavlov)
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 ?
(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.
Attachment #371239 - Flags: superreview?(pavlov) → superreview+
pushed. 8c64012db5ee

it should also land on 1.9.1 but I presume it has to be blocking-fennec1.0+ for that.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee: tonikitoo → nobody
Component: General → Widget: Win32
Product: Fennec → Core
QA Contact: general → win32
Attachment #371239 - Flags: approval1.9.1?
Assignee: nobody → tonikitoo
Component: Widget: Win32 → Widget: Gtk
QA Contact: win32 → gtk
Comment on attachment 371239 [details] [diff] [review]
v0.1 - rought patch that makes it work again ...

a191=beltzner
Attachment #371239 - Flags: approval1.9.1? → approval1.9.1+
Keywords: checkin-needed
Whiteboard: [needs 191 landing]
verified with maemo nightly 20090818
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.