Bug 85817 - [Qt] Implement WebProcess based input method event handling
Summary: [Qt] Implement WebProcess based input method event handling
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Brüning
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-07 12:48 PDT by Simon Hausmann
Modified: 2014-02-03 03:20 PST (History)
4 users (show)

See Also:


Attachments
Work in progress patch. (31.58 KB, patch)
2012-07-19 10:12 PDT, Michael Brüning
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hausmann 2012-05-07 12:48:32 PDT
The current handling of Qt Input Method Events (IMEs) tries to map the event parameters to either a SetComposition or ConfirmComposition message to the web process. Unfortunately this mapping is not entirely correct and breaks with certain virtual keyboards that use the IME interface.

A very concrete example: Imagine a virtual keyboard that makes suggestions based on what you write. So far you've written "Pla" and the keyboard suggests "Planet". The incomplete "Pla" sequence however is not a composition. Upon accepting the suggestion we receive an IME with "Planet" as commit string and (-3, 3) as replacementStart/Length.

In Qt the controls that respond the IMEs implement roughly the following "algorithm":

    (1) Remove the text referred to by replacementStart/Length
    (2) Insert the commit string
    (3) Set text selection as specified via QInputMethodEvent::Selection entry from attributes()
    (4) Set preeditText() as composition along with specified attributes

Any of the above steps are optional if their argument is not present - for example step (2) is a no-op if the commit string is empty. All of the above steps should appear as one operation in terms of undo/redo to the user. Step (1) and (2) can easily be combined into one edit operation.

I suggest we implement this algorithm in one shot on the web process side, to reduce the number of messages to be sent. I suggest to map the above steps to the WebCore::Editor interface as follows:

    * Step (1) and (2) become: Set replacementStart/Length as selection, convert it to composition with setComposition(selectedText()), confirmComposition(commitString)
    * Step (4) becomes a call to setComposition(preeditText)


On the UIProcess side we need to act on the IME synchronously, which means we need to apply the provided change simultaneously and immediately on the cached selection text (modify surroundingText, cursorPosition, etc.). We should also generate an integer ID, pass it as parameter with the IME message to the WebProcess and let the WebProcess send it back to us with one final EditorStateChanged message after applying steps (1)-(4). Until that EditorStateChanged message with the expected ID has arrived, we should ignore all incoming EditorStateChanged messages or at least not inform the input method about any updates, to avoid providing an intermediate (and thus incorrect) state to the input method.
Comment 1 Michael Brüning 2012-05-08 02:00:28 PDT
I'll start on a patch based on the branch from Simon's github.
Comment 2 Simon Hausmann 2012-06-27 00:41:10 PDT
Michael, is this still on your TODO or would you prefer we remove it from the 5.0.0 tracking?
Comment 3 Michael Brüning 2012-06-27 02:53:44 PDT
This is still on my todo and I'll continue working on this. I'll post a patch soon.
Comment 4 Michael Brüning 2012-07-19 10:12:50 PDT
Created attachment 153289 [details]
Work in progress patch.

I have been working on this on and off and since I am going on holidays, I wanted to post it here.

This is the current work in progress as of today, so it is not yet 100% ready for review. It seems to be working so far, although the unit test needs to be revisited. 

I am not 100% sure whether the UI process side is 100% correct and necessary. 

Also, there is still more editorState updates (multiply per event) dispatched, which I would like to solve, but haven't really found a good way to.
Comment 5 Simon Hausmann 2012-11-19 01:13:34 PST
Taking this off the 5.0.0 blocker because it shouldn't block the release.
Comment 6 Jocelyn Turcotte 2014-02-03 03:20:46 PST
=== 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.