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
What platform is this report about? WebKit on Mac completely ignores setInputMethodState() calls, so this cannot affect Safari.
PC/linux. I've jusr updated the bug record.
webkit gtk
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()
It's really confusing that cross-platform code in WebCore only affects some platforms, and is even against conventions of other ones.
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.
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").
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.
> 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.
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.
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).
(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.
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
Actually, this affects Epiphany as well. So Sugar and Epiphany would both profit from a fix here.
(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.
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.
Created attachment 386446 [details] Patch It won't apply, since it depends on bug #205605
Created attachment 387330 [details] Rebased patch
Committed r254675: <https://trac.webkit.org/changeset/254675>
<rdar://problem/58642844>