Bug 34285 – [GTK][WPE] Password field doesn't get input method

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Jaroslav Klaus]

Is it correct password field doesn't get input method?

In FocusController::setFocusedNode there is 
  m_page->editorClient()->setInputMethodState(node->shouldUseInputMethod());

while 
bool HTMLInputElement::shouldUseInputMethod() const
{
    return m_type == TEXT || m_type == SEARCH || m_type == ISINDEX;
}

So for password field setInputMethodState is always called with false that is problem for on-screen-keyboard.

Can you consider to extend  shouldUseInputMethod  to PASSWORD type? Thx

[Alexey Proskuryakov]

What platform is this report about? WebKit on Mac completely ignores setInputMethodState() calls, so this cannot affect Safari.

[Jaroslav Klaus]

PC/linux. I've jusr updated the bug record.

[Jaroslav Klaus]

webkit gtk

[Brian Tarricone]

Yes, changing that line does indeed fix it for the gtk port:

--- a/WebCore/html/HTMLInputElement.cpp
+++ b/WebCore/html/HTMLInputElement.cpp
@@ -523,7 +523,7 @@ void HTMLInputElement::aboutToUnload()
 
 bool HTMLInputElement::shouldUseInputMethod() const
 {
-    return m_type == TEXT || m_type == SEARCH || m_type == ISINDEX;
+    return m_type == TEXT || m_type == SEARCH || m_type == ISINDEX || m_type == PASSWORD;
 }
 
 void HTMLInputElement::handleFocusEvent()

[Alexey Proskuryakov]

It's really confusing that cross-platform code in WebCore only affects some platforms, and is even against conventions of other ones.

[Christian Dywan]

Created attachment 81956 [details]
Allow input methods in password input elements

This patch enables input methods in password entries. There may be security concerns, but it doesn't help if I want to use WebKit on a device without a physical way to type the password.

If there is strong concern, I can modify the patch to make this a setting.

[Alexey Proskuryakov]

This needs:

#if PLATFORM(MAC)
    // Platform convention is to only allow Roman characters in passwords.
    ASSERT_NOT_REACHED();
#endif

I would really prefer a deeper refactoring that wouldn't leave us with Gtk specific code in HTMLInputElement.

+    // Arguably insecure. In practise required if the password must be

We normally use American spelling in WebKit (i.e. "practice").

[Christian Dywan]

Created attachment 82130 [details]
Allow input methods in password input elements, #2

(In reply to comment #7)
> #if PLATFORM(MAC)
>     // Platform convention is to only allow Roman characters in passwords.

This is not just about non-Latin characters, but also virtual keyboards, thusly I made it:

// Platform convention is to disallow input methods for passwords.

> I would really prefer a deeper refactoring that wouldn't leave us with Gtk specific code in HTMLInputElement.

I don't have a Mac and I'm probably not a good person to judge how it would need to be changed. From a quick look I didn't even see password handling at all, so I assume it relies on system behaviour to a good extend.

> +    // Arguably insecure. In practise required if the password must be
> 
> We normally use American spelling in WebKit (i.e. "practice").

Corrected.

[Alexey Proskuryakov]

> This is not just about non-Latin characters, but also virtual keyboards, thusly I made it:

Please let me reiterate that virtual keyboard is not an input method on Mac, and works with password fields just great. Generally, it sounds more like an exception than a rule that on-screen keyboards would use input method machinery.

Your new comment seems fine though.

> I don't have a Mac

I've just noticed that even though this function has no effect whatsoever on Mac, it does affect Safari for Windows, quite possibly in a bad way. Thus, deferring to Adam or Enrica for review.

[Adam Roben (:aroben)]

I guess someone should test on Windows to see what the platform convention is. What do password fields in the OS do? What do other browsers do? I'm not familiar with input methods, so I don't know how to perform this testing myself.

[Martin Robinson]

Comment on attachment 82130 [details]
Allow input methods in password input elements, #2

View in context: https://bugs.webkit.org/attachment.cgi?id=82130&action=review

I believe that in Chromium, they only allow simple input method events.

> Source/WebCore/html/PasswordInputType.cpp:67
> +#if PLATFORM(MAC)
> +    // Platform convention is to disallow input methods for passwords.
> +    ASSERT_NOT_REACHED();
> +#endif

This seems wrong. Why should PLATFORM(MAC) ASSERT_NOT_REACHED() ?

> Source/WebCore/html/PasswordInputType.cpp:70
> +    return true;

If we going to do this, we probably only want to do it for PLATFORM(GTK).

[Christian Dywan]

(In reply to comment #11)
> > Source/WebCore/html/PasswordInputType.cpp:67
> > +#if PLATFORM(MAC)
> > +    // Platform convention is to disallow input methods for passwords.
> > +    ASSERT_NOT_REACHED();
> > +#endif
> 
> This seems wrong. Why should PLATFORM(MAC) ASSERT_NOT_REACHED() ?
> 
> > Source/WebCore/html/PasswordInputType.cpp:70
> > +    return true;
> 
> If we going to do this, we probably only want to do it for PLATFORM(GTK).

Please see the previous comments, I did exactly what you are suggesting originally and the feedback was to do it for all ports except Mac which has a different mechanism.

[Simon Schampijer]

Hmm, this one is important for OLPC (who uses webkitgtk). At the moment you can not log into any page that has a passoword field using the on screen keyboard.

Can the patch provided on this ticket be revised another time and included in some form? There seem to have been mostly agreement and things seems stuck in the last 10% :/

For reference: http://bugs.sugarlabs.org/ticket/4069

[Simon Schampijer]

Actually, this affects Epiphany as well. So Sugar and Epiphany would both profit from a fix here.

[Carlos Garcia Campos]

(In reply to Martin Robinson from comment #11)
> Comment on attachment 82130 [details]
> Allow input methods in password input elements, #2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=82130&action=review
> 
> I believe that in Chromium, they only allow simple input method events.

Both chromium and firefox allow input methods in password fields.

[Carlos Garcia Campos]

After bug #205605 we will notify the input methods about the editable purpose, so for password fields we will set the purpose as WEBKIT_INPUT_PURPOSE_PASSWORD. That way the input methods (including virtual keyboards) can ensure the characters won't be visible and be prepared for a password input.

[Carlos Garcia Campos]

Created attachment 386446 [details]
Patch

It won't apply, since it depends on bug #205605

[Carlos Garcia Campos]

Created attachment 387330 [details]
Rebased patch

[Carlos Garcia Campos]

Committed r254675: <https://trac.webkit.org/changeset/254675>

[Radar WebKit Bug Importer]

<rdar://problem/58642844>