Bug 60161 – [Qt] Virtual Keyboard is not hidden when you 'unfocus' from input field / QGraphicsWebview

• Component: WebKit Qt
• Status: RESOLVED INVALID

[Leonard Lee]

Attached test application.

On any QML WebView page, ie pointing to google.com

- Press your finger to some input field
- VKB should come up (and it does)
- Type something on the input field.
- Press your finger outside of the input field (unfocused).

EXPECTED OUTCOME:
VKB should be hidden on unfocused.

ACTUAL OUTCOME:
VKB stays visible on unfocused.

[Benjamin Poulain]

Kenneth, you played with that IIRC, any comment?

[Kenneth Rohde Christiansen]

(In reply to comment #1)
> Kenneth, you played with that IIRC, any comment?

Yes, and we use a totally different path for mobile input handling on our branch and used on the desktop, as they are very different use cases. At some point I would like to upstream that code.

[Leonard Lee]

(In reply to comment #2)
> (In reply to comment #1)
> > Kenneth, you played with that IIRC, any comment?
> 
> Yes, and we use a totally different path for mobile input handling on our branch and used on the desktop, as they are very different use cases. At some point I would like to upstream that code.

Is this issue related to what you've experience? If you upstream the code, would that fix this issue?

[Benjamin Poulain]

I can confirm the issue with QtTestBrowser. However I set low priority to reflect that we have nobody working on this, Kenneth has other priorities at the moment.

[Topi Reiniö]

(In reply to comment #4)
> I can confirm the issue with QtTestBrowser. However I set low priority to reflect that we have nobody working on this, Kenneth has other priorities at the moment.

This is quite bad for 3rd party apps, since it affects everything (QML WebView, QGraphicsWebView and QWebView). Is this still stuck in the backlog?

I created a clumsy workaround for this using an event filter and sending CloseSoftwareInputPanel events whenever the focused widget changes to zero:

class EventFilter : public QObject
{   
protected:
    bool eventFilter(QObject *obj, QEvent *event) {
        QInputContext *ic = qApp->inputContext();
        if (ic) {
            if (ic->focusWidget() == 0 && prevFocusWidget) {
                QEvent closeSIPEvent(QEvent::CloseSoftwareInputPanel);
                ic->filterEvent(&closeSIPEvent);
            } else if (prevFocusWidget == 0 && ic->focusWidget()) {
                QEvent openSIPEvent(QEvent::RequestSoftwareInputPanel);
                ic->filterEvent(&openSIPEvent);
            }
            prevFocusWidget = ic->focusWidget();
        }
        return QObject::eventFilter(obj,event);
    }

private:
    QWidget *prevFocusWidget;
};

This will also do the reverse, i.e, request SIP to open when focus widget
changes to non-null, resulting in a single-tap VKB popup.

This filter can be then installed on a QGraphicsView or (in QML) on a
QDeclarativeView instance:

EventFilter ef;
view.installEventFilter(&ef);


But of course, to have a consistent user experience, it would be way better to have this fixed directly in webkit.

[Kenneth Rohde Christiansen]

We should do it the proper way instead. Maybe one from INDT can look into it if it is considered important for the next QtWebKit release? cc'ing Luiz and Caio.

[Topi Reiniö]

Created attachment 111254 [details]
Patch proposal to somewhat fix the issue

Copying here the patch attached to the originating bug.

-- 

[PATCH] Close software input panel on element blur.

Send an explicit event to open and close software input panel when
input elements are focused and unfocused.

Qt software input framework works through QWidgets. In case of
QGraphicsWebView, the QGraphicsView that contains QGraphicsWebView
stays focused, regardless whether the QGraphicsItems in it has focus
or not.  This means that SIP is never closed, even though the
QGraphicsWebView loses focus.

The framework expects that SIP state is controlled explicitly.

This commit fixes also various problems when focus is controlled
through JS.

FIXME: Might cause regression with QStyle -related SIP appearance
configuration. It is unclear whether the previous code ever worked.

[Bruno Abinader (history only)]

Created attachment 112688 [details]
"Close software input panel on element blur" patch.

Updated patch with missing ChangeLog and fixed style issues.

[Kenneth Rohde Christiansen]

Comment on attachment 112688 [details]
"Close software input panel on element blur" patch.

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

> Source/WebCore/ChangeLog:1
> [phone]  Bruno de Oliveira Abinader  <[e-mail]>

Seems like you changed company :-) congrats! Living in Germany now?

> Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp:202
> +        QApplication::sendEvent(focusedWidget, &openSIPEvent);

Shouldn't you use filterEvent here? This is what is done on MeeGo

> Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp:213
> -    view->setAttribute(Qt::WA_InputMethodEnabled, enable);
> +    if (enable)
> +        view->setAttribute(Qt::WA_InputMethodEnabled, enable);

I don't think this is supposed to be turned on and off when showing/hiding the vkb. The widget supports IM, so it should just be enabled

[Kenneth Rohde Christiansen]

Simon, can you have a look?

[Simon Hausmann]

Comment on attachment 112688 [details]
"Close software input panel on element blur" patch.

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

I'm going to say r- because there are a few open questions here. Support for QStyle::RSIP_OnMouseClick was removed and it's not clear why QApplication::focusWidget() should be preferred over more specific item/widget hasFocus() functions.

Also I don't see how this patch handles the case where you just "unblur" an element within the page. AFAICS this patch only calls updateSoftwareInputPanel from QWebPage::focusInEvent, which is certainly not called when a focusable node within the document looses focus.

In the N9 we solved this through EditorClient::setInputMethodState, which through a lengthy chain of calls should end up sending the RSIP _or_ the CSIP event.

> Source/WebKit/qt/Api/qwebpage.cpp:-813
> -        QStyle::RequestSoftwareInputPanel behavior = QStyle::RequestSoftwareInputPanel(
> -            client->ownerWidget()->style()->styleHint(QStyle::SH_RequestSoftwareInputPanel));
> -        if (!clickCausedFocus || behavior == QStyle::RSIP_OnMouseClick) {

So _this_ is the part that your patch removes entirely, the support for QStyle::RSIP_OnMouseClick. Why did you remove it?

> Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp:196
> +    QWidget* focusedWidget = QApplication::focusWidget();
> +
> +    if (!focusedWidget)
> +        return;
> +

That doesn't look quite right. Wouldn't it be cleaner to check for view->hasFocus() and also send the RSIP/CSIP events to view?

> Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp:413
> +    QGraphicsView* focusedGraphicsView = qobject_cast<QGraphicsView*>(QApplication::focusWidget());
> +
> +    if (focusedGraphicsView
> +        && view->hasFocus()

Similarly here I think it would be better to use QGraphicsItem::hasFocus() instead of using QApplication::focusWidget().

> Source/WebKit/qt/WebCoreSupport/PageClientQt.cpp:435
> +    if (enable)
> +        view->setFlag(QGraphicsItem::ItemAcceptsInputMethod, true);
> +
> +    updateSoftwareInputPanel(enable);
> +
> +    if (!enable)
> +        view->setFlag(QGraphicsItem::ItemAcceptsInputMethod, false);

A comment here (as well as in the other method) would probably help to understand why
you enable before calling updateSoftwareInputPanel and disable afterwards.

[Bruno Abinader (history only)]

Hi guys,

First of all, thanks for the reviews. I have been studying the SIP behavior all along different applications on Harmattan. On grob browser, for instance, the behavior is correct. On QML TextInput, you can actually disable the default behavior when setting "activeFocusOnPress" to false and implement a custom VKB call using openSoftwareInputPanel() and closeSoftwareInputPanel() methods. On all these cases, I've enabled maliit debugging mode (export MIC_ENABLE_DEBUG=true) and this is what I get:

    MInputContext got CloseSoftwareInputPanel event 
    MInputContext in virtual void MInputContext::reset() 
    MInputContext in virtual void MInputContext::setFocusWidget(QWidget*) QObject(0x0)

However, without this patch, here's what output on console when "unfocus" from the text input widget:

    MInputContext in virtual void MInputContext::update() 
    MInputContext in virtual void MInputContext::reset() 
    MInputContext in virtual void MInputContext::setFocusWidget(QWidget*) QObject(0x0)

So indeed there is no call for CloseSoftwareInputPanel event anywhere in QWebPage. I've looked at grob code and the IM behavior is based on the editFocusChanged(QRect) signal provided by QWKPage. There is no such signal on current QtWebKit version.

That said, please disconsider this patch. I'm now currently testing a small modification of mine on QWebPagePrivate::handleSoftwareInputPanel that might fix the issue without removing any code or adding more virtual functions. If that doesn't work, there's still an option to add an event filter on both
PageClient{QWidget,QGraphicsWidget}. I'm going to publish it here as soon as I have a working patch.

[Bruno Abinader (history only)]

Created attachment 113501 [details]
"Close SIP explicitly when IM is disabled" patch.

This updated patch does not remove any support for QStyle::RSIP_OnMouseClick (indeed, first click on the input text box focuses it, but only after second click the VKB appears, same expected behavior). The static bool sipRequested is just a flag to avoid unnecessary emissions of QEvent::CloseSoftwareInputPanel. Also, as Kenneth suggested, filterEvent() is used to propagate the event.

ps: While working on this patch, I noticed, for example, that the VKB is still not closed after you digit anything on Google's text box and click on "Return" key. This updates the view screen showing the results of Google's query, so focus is no longer at the text box. I tried adding some logic when the "loadFinished" signal is emitted from QWebView and it worked, but with some issues still to resolve. I am going to create another bug entry for this particular case.

[Kenneth Rohde Christiansen]

Comment on attachment 113501 [details]
"Close SIP explicitly when IM is disabled" patch.

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

> Source/WebKit/qt/Api/qwebpage.cpp:820
> +    if (!client->inputMethodEnabled()) {
> +        QInputContext *inputContext = qApp->inputContext();
> +        if (inputContext) {
> +            inputContext->reset();
> +            inputContext->update();

If it is not enabled you do stuff?

> Source/WebKit/qt/Api/qwebpage.cpp:822
> +            //QEvent closeSIPEvent(QEvent::CloseSoftwareInputPanel);
> +            //inputContext->filterEvent(&closeSIPEvent);

Patches should not add commented out code

[WebKit Review Bot]

Attachment 113501 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/Api/qwebpage.cpp', u'Sour..." exit_code: 1

Source/WebKit/qt/Api/qwebpage.cpp:813:  Declaration has space between type name and * in QInputContext *inputContext  [whitespace/declaration] [3]
Source/WebKit/qt/Api/qwebpage.cpp:817:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebKit/qt/Api/qwebpage.cpp:818:  Should have a space between // and comment  [whitespace/comments] [4]
Source/WebKit/qt/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Bruno Abinader (history only)]

Created attachment 113502 [details]
Updated "Close SIP explicitly when IM is disabled" patch.

Deep sorry guys, I made a mistake when sending the attachment. But taking the opportunity to answering Kenneth's question: Yes, the QEvent::CloseSoftwareInputPanel event is sent if client->inputMethodEnabled() returns false. The rationale is that if IM is disabled, then there's no need anymore to show SIP. I've tested it against the testcase and works as expected.

[Bruno Abinader (history only)]

Comment on attachment 113502 [details]
Updated "Close SIP explicitly when IM is disabled" patch.

Assigning "?" to "review" and "commit-queue" flags.

[WebKit Review Bot]

Attachment 113502 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/Api/qwebpage.cpp', u'Sour..." exit_code: 1

Source/WebKit/qt/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Bruno Abinader (history only)]

Created attachment 113505 [details]
ChangeLog-updated "Close SIP explicitly when IM is disabled" patch.

Updated patch ChangeLog with [Qt] tag and WebKit bugzilla entry.

[Simon Hausmann]

Comment on attachment 113505 [details]
ChangeLog-updated "Close SIP explicitly when IM is disabled" patch.

r-

As just discussed on IRC, the CSIP event should not be necessary to send, as the input context should hide the vkb when the input method attribute/flag gets disabled.

Alternatively if that's unfixible on the input method side, the CSIP event should be sent where we disable the flag.

The ChangeLog also says OOPS and the sipRequested variable is unused :)

[Bruno Abinader (history only)]

Comment on attachment 113505 [details]
ChangeLog-updated "Close SIP explicitly when IM is disabled" patch.

Removing "review" and "commit-queue" patches.

[Bruno Abinader (history only)]

As discussed with Simon via IRC, this is a Harmattan-only issue, due to a misbehavior from MInputContext, so not any longer an upstream issue.

[Mattia Barbon]

(In reply to comment #20)
> (From update of attachment 113505 [details])

I'm having te same issue when using Maliit with Qt/Webkit in a QML WebView (tested with Qt 4.8.0), and also the issue mentioned in comment #13.

> As just discussed on IRC, the CSIP event should not be necessary to send, as the input context should hide the vkb when the input method attribute/flag gets disabled.

Is this the Qt::WA_InputMethodEnabled flag or is it something else?

> Alternatively if that's unfixible on the input method side, the CSIP event should be sent where we disable the flag.

Could you point me to the IRC logs for the conversation mentioned above?

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.