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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60161


Simon Hausmann <hausmann at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #112688|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #11 from Simon Hausmann <hausmann at webkit.org>  2011-10-28 02:47:07 PST ---
(From update of attachment 112688)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.