Bug 40107 - Impossible to set input method hints based HTML5 input types
Summary: Impossible to set input method hints based HTML5 input types
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Raine Makelainen
URL:
Keywords: Qt
: 39676 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-06-03 01:09 PDT by Raine Makelainen
Modified: 2011-04-19 05:15 PDT (History)
9 users (show)

See Also:


Attachments
Proposal to set input method hints for EditorClientQt (16.31 KB, patch)
2010-06-08 03:13 PDT, Raine Makelainen
no flags Details | Formatted Diff | Diff
2nd proposal for fix. (14.66 KB, patch)
2010-06-09 08:08 PDT, Raine Makelainen
no flags Details | Formatted Diff | Diff
3rd proposal for fix. (14.51 KB, patch)
2010-06-09 13:48 PDT, Raine Makelainen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Raine Makelainen 2010-06-03 01:09:58 PDT
When HTML5 input element is focused on a touch screen based environment, the correct virtual keyboard for the particular input should be selected.
Comment 1 Raine Makelainen 2010-06-03 01:30:16 PDT
(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.
Comment 2 Kenneth Rohde Christiansen 2010-06-03 05:14:12 PDT
Sounds good! Im not on irc the next two days (holiday here) but feel free to email me.
Comment 3 Raine Makelainen 2010-06-08 03:13:07 PDT
Created attachment 58126 [details]
Proposal to set input method hints for EditorClientQt
Comment 4 Raine Makelainen 2010-06-08 03:16:07 PDT
Darin to CC. There are some changes to HTMLInputElement.
Comment 5 Antonio Gomes 2010-06-08 07:26:43 PDT
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 <raine.makelainen@nokia.com>

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.
Comment 6 Raine Makelainen 2010-06-09 06:08:40 PDT
Removing [Qt] from title.
Comment 7 Raine Makelainen 2010-06-09 08:08:06 PDT
Created attachment 58246 [details]
2nd proposal for fix.

Commit log entry removed and change logs updated.
Comment 8 Kenneth Rohde Christiansen 2010-06-09 08:21:32 PDT
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.
Comment 9 Kent Tamura 2010-06-09 08:46:27 PDT
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.
Comment 10 Kenneth Rohde Christiansen 2010-06-09 08:54:25 PDT
(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
Comment 11 Kent Tamura 2010-06-09 09:11:50 PDT
(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.
Comment 12 Kenneth Rohde Christiansen 2010-06-09 09:35:47 PDT
> 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.
Comment 13 Raine Makelainen 2010-06-09 12:21:07 PDT
(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.
Comment 14 Raine Makelainen 2010-06-09 13:48:24 PDT
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
Comment 15 Eric Seidel (no email) 2010-06-10 01:25:12 PDT
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.
Comment 16 WebKit Commit Bot 2010-06-10 08:34:16 PDT
Comment on attachment 58291 [details]
3rd proposal for fix.

Clearing flags on attachment: 58291

Committed r60958: <http://trac.webkit.org/changeset/60958>
Comment 17 WebKit Commit Bot 2010-06-10 08:34:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Simon Hausmann 2010-06-18 03:03:44 PDT
cherry-picked into qtwebkit-4.6 with commit 85a48bdb52a81a9d18477a347fba5f6c930af416
Comment 19 Simon Hausmann 2010-06-18 06:03:37 PDT
Revision r60958 cherry-picked into qtwebkit-2.0 with commit ab885e9fdbd7572bbb400b3c7ee7b51eb5b49e86
Comment 20 Janne Koskinen 2010-11-22 04:13:30 PST
*** Bug 39676 has been marked as a duplicate of this bug. ***