Bug 40107 – Impossible to set input method hints based HTML5 input types

• Component: WebKit Qt
• Status: CLOSED FIXED

[Raine Makelainen]

When HTML5 input element is focused on a touch screen based environment, the correct virtual keyboard for the particular input should be selected.

[Raine Makelainen]

(In reply to comment #0)
> When HTML5 input element is focused on a touch screen based environment, the correct virtual keyboard for the particular input should be selected.

I'd like start working on this with following approach:
- Adding EditorClientQt to support following input types: "number", "tel".
- Also some helper methods should be added to WebCore/html/HTMLInputElement

Thanks, Kenneth for feedback.

[Kenneth Rohde Christiansen]

Sounds good! Im not on irc the next two days (holiday here) but feel free to email me.

[Raine Makelainen]

Created attachment 58126 [details]
Proposal to set input method hints for EditorClientQt

[Raine Makelainen]

Darin to CC. There are some changes to HTMLInputElement.

[Antonio Gomes]

Comment on attachment 58126 [details]
Proposal to set input method hints for EditorClientQt

some comments:

> Helper methods for checking "tel", "number", "email",
> and "url" input element types for HTMLInputElement.h.
> 
> EditorClientQt to support input method hints for "number", "tel",
> "email", and "url" HTML input elements.
> 
> Tests for HTML input elements and input method hints added for
> QGraphicsWebView and QWebView.

Please make you commit message based on the ChangeLog contents. It generally also has Reviewed by and bug url lines.

> Signed-off-by: Raine Makelainen <[e-mail]>

Signed-off it is not used in WebKit , but generally "Patch by XYZ" just after "Reviewed by ABC".

> diff --git a/WebCore/html/HTMLInputElement.h b/WebCore/html/HTMLInputElement.h
> index 363a25f..2527e8b 100644
> --- a/WebCore/html/HTMLInputElement.h
> +++ b/WebCore/html/HTMLInputElement.h
> @@ -101,6 +101,10 @@ public:
>      virtual bool isSearchField() const { return m_type == SEARCH; }
>      virtual bool isInputTypeHidden() const { return m_type == HIDDEN; }
>      virtual bool isPasswordField() const { return m_type == PASSWORD; }
> +    virtual bool isTelephoneField() const { return m_type == TELEPHONE; }
> +    virtual bool isNumberField() const { return m_type == NUMBER; }
> +    virtual bool isEmailField() const { return m_type == EMAIL; }
> +    virtual bool isUrlField() const { return m_type == URL; }

You are touching a cross platform code, so bug automatically is not Qt-specific, but can rather have the 'Qt' keyword in it.

It should probably be a separate bug, blocking this one(?)

Good autotests, btw.

[Raine Makelainen]

Removing [Qt] from title.

[Raine Makelainen]

Created attachment 58246 [details]
2nd proposal for fix.

Commit log entry removed and change logs updated.

[Kenneth Rohde Christiansen]

Comment on attachment 58246 [details]
2nd proposal for fix.


> +            if (frame->document()->focusedNode()->hasTagName(HTMLNames::inputTag)) {
> +                inputElement = static_cast<HTMLInputElement*>(frame->document()->focusedNode());
>              }

Should not use braces.


> +                bool isPasswordField = false;
> +                active = isPasswordField = inputElement->isPasswordField();

The - false here is useless.

> +                webPageClient->setInputMethodHint(Qt::ImhHiddenText, isPasswordField);
> +            } else {
> +                // Set input method hints for "number", "tel", "email", and "url" input elements.
> +                webPageClient->setInputMethodHint(Qt::ImhDialableCharactersOnly, inputElement->isTelephoneField());
> +                webPageClient->setInputMethodHint(Qt::ImhDigitsOnly, inputElement->isNumberField());
> +                webPageClient->setInputMethodHint(Qt::ImhEmailCharactersOnly, inputElement->isEmailField());
> +                webPageClient->setInputMethodHint(Qt::ImhUrlCharactersOnly, inputElement->isUrlField());

Are *ALL* of these defined for Qt 4.6 ? If not, we need some ifdefs. I do not believe that Email is available for instance.

r=me with these fixes.

[Kent Tamura]

Comment on attachment 58246 [details]
2nd proposal for fix.

WebCore/html/HTMLInputElement.h:107
 +      virtual bool isUrlField() const { return m_type == URL; }
I don't think these new functions should be virtual.

[Kenneth Rohde Christiansen]

(In reply to comment #9)
> (From update of attachment 58246 [details])
> WebCore/html/HTMLInputElement.h:107
>  +      virtual bool isUrlField() const { return m_type == URL; }
> I don't think these new functions should be virtual.

The already existing ones are virtual though. Maybe we should change those as well? or there is a reason why they are virtual

[Kent Tamura]

(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 58246 [details] [details])
> > WebCore/html/HTMLInputElement.h:107
> >  +      virtual bool isUrlField() const { return m_type == URL; }
> > I don't think these new functions should be virtual.
> 
> The already existing ones are virtual though. Maybe we should change those as well? or there is a reason why they are virtual

The existing isFoo() functions override functions of dom/InputElement, and they are virtual. wml/WMLinputElement overrides them too.

If we have no reasons to add the new functions to dom/InputElement, we should remove virtual from the new functions.

[Kenneth Rohde Christiansen]

> The existing isFoo() functions override functions of dom/InputElement, and they are virtual. wml/WMLinputElement overrides them too.
> 
> If we have no reasons to add the new functions to dom/InputElement, we should remove virtual from the new functions.

I agree. Raine, please fix this before committing.

[Raine Makelainen]

(In reply to comment #8)
> (From update of attachment 58246 [details])
> 
> > +                bool isPasswordField = false;
> > +                active = isPasswordField = inputElement->isPasswordField();
> 
> The - false here is useless.
Right, actually whole variable is not needed. Variable active is enough. I'll clean this.

> 
> > +                webPageClient->setInputMethodHint(Qt::ImhHiddenText, isPasswordField);
> > +            } else {
> > +                // Set input method hints for "number", "tel", "email", and "url" input elements.
> > +                webPageClient->setInputMethodHint(Qt::ImhDialableCharactersOnly, inputElement->isTelephoneField());
> > +                webPageClient->setInputMethodHint(Qt::ImhDigitsOnly, inputElement->isNumberField());
> > +                webPageClient->setInputMethodHint(Qt::ImhEmailCharactersOnly, inputElement->isEmailField());
> > +                webPageClient->setInputMethodHint(Qt::ImhUrlCharactersOnly, inputElement->isUrlField());
> 
> Are *ALL* of these defined for Qt 4.6 ? If not, we need some ifdefs. I do not believe that Email is available for instance.

According to documentation we have them all for Qt 4.6:
http://doc.trolltech.com/4.6/qt.html#InputMethodHint-enum


> 
> r=me with these fixes.

[Raine Makelainen]

Created attachment 58291 [details]
3rd proposal for fix.

Review comments merged:
- Unnecessary braces removed
- isPasswordField bool variable removed (active is enough)
- New helper methods in HTMLInputElement defined without virtual

[Eric Seidel (no email)]

Comment on attachment 58246 [details]
2nd proposal for fix.

Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 58246 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[WebKit Commit Bot]

Comment on attachment 58291 [details]
3rd proposal for fix.

Clearing flags on attachment: 58291

Committed r60958: <http://trac.webkit.org/changeset/60958>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

cherry-picked into qtwebkit-4.6 with commit 85a48bdb52a81a9d18477a347fba5f6c930af416

[Simon Hausmann]

Revision r60958 cherry-picked into qtwebkit-2.0 with commit ab885e9fdbd7572bbb400b3c7ee7b51eb5b49e86

[Janne Koskinen]

*** Bug 39676 has been marked as a duplicate of this bug. ***