Bug 67344 - [Qt] Transform QtFallbackWebPopupCombo into QtWebComboBox
Summary: [Qt] Transform QtFallbackWebPopupCombo into QtWebComboBox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Marcelo de Oliveira Filho
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 62191
  Show dependency treegraph
 
Reported: 2011-08-31 17:08 PDT by Caio Marcelo de Oliveira Filho
Modified: 2011-09-01 11:38 PDT (History)
3 users (show)

See Also:


Attachments
Patch (14.94 KB, patch)
2011-08-31 17:25 PDT, Caio Marcelo de Oliveira Filho
kling: review+
kling: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Marcelo de Oliveira Filho 2011-08-31 17:08:30 PDT
Create a replacement for QtFallbackWebPopupCombo that can be used also used by WK2 port.
Comment 1 Caio Marcelo de Oliveira Filho 2011-08-31 17:25:55 PDT
Created attachment 105868 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2011-09-01 06:37:00 PDT
Comment on attachment 105868 [details]
Patch

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

> Source/WebKit/qt/WebCoreSupport/QtWebComboBox.cpp:64
> +    // This workaround was added in r40970 to address a problem in Qt. I _think_ this problem
> +    // was solved in commit 3ab68cbf5436e3e66d0297b01af661bedecb8766 of Qt4 repository.
> +#ifndef QT_NO_IM
> +    QWidget* activeFocus = QApplication::focusWidget();
> +    if (activeFocus && activeFocus == QComboBox::view()
> +        && activeFocus->testAttribute(Qt::WA_InputMethodEnabled)) {
> +        QInputContext* qic = activeFocus->inputContext();
> +        if (qic) {
> +            qic->reset();
> +            qic->setFocusWidget(0);
> +        }
> +    }
> +#endif // QT_NO_IM

I would just remove this, it is broken anyway and might bite us. It is quite dangeous resetting the input context behind our backs as it can easily get out of sync, for instance if I have a composition, then this would reset that from the internal state of the input context and it will get our of sync with webcore
Comment 3 Andreas Kling 2011-09-01 07:33:14 PDT
Comment on attachment 105868 [details]
Patch

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

Beautiful refactor! r=me

> Source/WebKit/qt/ChangeLog:11
> +        signal) and a convenience to show its popup in the cursor position.

convenience method*

>> Source/WebKit/qt/WebCoreSupport/QtWebComboBox.cpp:64
>> +#endif // QT_NO_IM
> 
> I would just remove this, it is broken anyway and might bite us. It is quite dangeous resetting the input context behind our backs as it can easily get out of sync, for instance if I have a composition, then this would reset that from the internal state of the input context and it will get our of sync with webcore

Agreed.

> Source/WebKit/qt/WebCoreSupport/QtWebComboBox.h:37
> +    void showPopupInCursorPosition();

Would sound slightly better as showPopupAtCursorPosition().
Comment 4 Caio Marcelo de Oliveira Filho 2011-09-01 11:38:27 PDT
Committed r94321: <http://trac.webkit.org/changeset/94321>