Bug 76169

Summary: [Qt] Set the input method hints on the QtQuick item
Product: WebKit Reporter: Kenneth Rohde Christiansen <kenneth>
Component: WebKit2Assignee: Kenneth Rohde Christiansen <kenneth>
Status: RESOLVED FIXED    
Severity: Normal CC: hausmann, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch hausmann: review+

Description Kenneth Rohde Christiansen 2012-01-12 04:58:42 PST
SSIA
Comment 1 Kenneth Rohde Christiansen 2012-01-12 05:00:03 PST
Created attachment 122216 [details]
Patch
Comment 2 Simon Hausmann 2012-01-12 05:36:10 PST
Comment on attachment 122216 [details]
Patch

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

> Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:439
> +    m_webPage->setInputMethodHints(Qt::InputMethodHints(editor.inputMethodHints));

Is the page the right object to set the hints on? We receive the input method event on the view, the view has the focus, so we should also be setting the input method hints on the view instead of on the page.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:420
> +        } else if (input->isPasswordField()) {
> +            // Set ImhHiddenText flag for password fields. The Qt platform
> +            // is responsible for determining which widget will receive input
> +            // method events for password fields.
> +            result.inputMethodHints |= Qt::ImhHiddenText;
> +            result.inputMethodHints |= Qt::ImhNoAutoUppercase;
> +            result.inputMethodHints |= Qt::ImhNoPredictiveText;

I think password fields in QML2 will also set the ImhSensitiveData flag.
Comment 3 Kenneth Rohde Christiansen 2012-01-12 06:18:47 PST
Created attachment 122228 [details]
Patch
Comment 4 Kenneth Rohde Christiansen 2012-01-12 06:43:15 PST
Created attachment 122230 [details]
Patch
Comment 5 Simon Hausmann 2012-01-12 06:46:37 PST
Comment on attachment 122230 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:80
> +    eventHandler.reset(new QtWebPageEventHandler(toAPI(webPageProxy), q, viewportItem));

r=me, but the naming seems a little inconsistent here. In here it's called the viewportItem and in the event handler it's the webview. I suggest to go for the latter all the way :)
Comment 6 Kenneth Rohde Christiansen 2012-01-12 06:52:20 PST
(In reply to comment #5)
> (From update of attachment 122230 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122230&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:80
> > +    eventHandler.reset(new QtWebPageEventHandler(toAPI(webPageProxy), q, viewportItem));
> 
> r=me, but the naming seems a little inconsistent here. In here it's called the viewportItem and in the event handler it's the webview. I suggest to go for the latter all the way :)

It is consisten with the naming in the current files :-/ but the naming is inconsistent across files, but it seems like something to fix in a separate patch
Comment 7 Kenneth Rohde Christiansen 2012-01-12 06:57:56 PST
Landed in 104823.