Bug 38779

Summary: [Qt] QWebPage::inputMethodQuery() returns wrong values for Qt::ImCursorPosition, Qt::ImAnchorPosition
Product: WebKit Reporter: rajiv.ramanasankaran
Component: WebKit QtAssignee: rajiv.ramanasankaran
Status: CLOSED FIXED    
Severity: Normal CC: commit-queue, hausmann, jesus, laszlo.gombos, rajiv.ramanasankaran, robert
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 35784    
Attachments:
Description Flags
Prosposed patch to fix the return values for Qt::ImCursorPosition and Qt::ImAnchorPosition in QWebPage::inputMethodQuery() when the Editor doesn't have a composition. none

Description rajiv.ramanasankaran 2010-05-07 15:55:37 PDT
QWebPage::inputMethodQuery(Qt::ImCursorPosition) returns the end index (renderTextControl->selectionEnd()) of a selection inside an input field instead of the current cursor position. 

QWebPage::inputMethodQuery(Qt::ImAnchorPosition) returns the start index (renderTextControl->selectionStart()) of a selection inside an input field instead of the anchor position.

To be specific: In Webkit, the START and END indices always correspond to the left and right edges of a selection irrespective of the current cursor direction. The bug is that the current implementation of QWebPage::inputMethodQuery() assumes incorrectly that the anchor position is the START index and that the current cursor position always corresponds to the END index.
Comment 1 rajiv.ramanasankaran 2010-05-07 15:58:19 PDT
I am working on a patch for submission.
Comment 2 Jesus Sanchez-Palencia 2010-05-12 07:28:06 PDT
(In reply to comment #1)
> I am working on a patch for submission.

Please consider adding an auto-test to it.
Comment 3 rajiv.ramanasankaran 2010-05-12 09:30:54 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > I am working on a patch for submission.
> Please consider adding an auto-test to it.

Thanks! Just to clarify:
I am working on a patch (including the fix and auto-tests) for submission.
Comment 4 rajiv.ramanasankaran 2010-05-16 19:47:14 PDT
Created attachment 56206 [details]
Prosposed patch to fix the return values for Qt::ImCursorPosition and Qt::ImAnchorPosition in QWebPage::inputMethodQuery() when the Editor doesn't have a composition.

The patch fixes the issue of incorrect values been returned for Qt::ImCursorPosition and Qt::ImAnchorPosition in QWebPage::inputMethodQuery() when the Editor doesn't have a composition.
Comment 5 Robert Hogan 2010-05-17 12:13:56 PDT
So is it not solvable when in Composition mode? Or not relevant? (Not a loaded question, genuinely don't know.)
Comment 6 rajiv.ramanasankaran 2010-05-17 12:49:35 PDT
(In reply to comment #5)
> So is it not solvable when in Composition mode? Or not relevant? (Not a loaded question, genuinely don't know.)

Sorry I wasn't specific. The values seem to be correct in Composition mode. I don't think there is a bug in that case.
Comment 7 Robert Hogan 2010-05-17 13:50:03 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > So is it not solvable when in Composition mode? Or not relevant? (Not a loaded question, genuinely don't know.)
> 
> Sorry I wasn't specific. The values seem to be correct in Composition mode. I don't think there is a bug in that case.

Funny, tst_qwebpage has:

    //Set selection with negative length
    inputAttributes << QInputMethodEvent::Attribute(QInputMethodEvent::Selection, 6, -5, QVariant());
    QInputMethodEvent eventSelection3("",inputAttributes);
    page->event(&eventSelection3);

    //ImAnchorPosition
    variant = page->inputMethodQuery(Qt::ImAnchorPosition);
    anchorPosition =  variant.toInt();
    QCOMPARE(anchorPosition, 1);

    //ImCursorPosition
    variant = page->inputMethodQuery(Qt::ImCursorPosition);
    cursorPosition =  variant.toInt();
    QCOMPARE(cursorPosition, 6);

which would suggest the opposite to me, but I don't know too much about input methods.
Comment 8 rajiv.ramanasankaran 2010-05-17 14:17:23 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > So is it not solvable when in Composition mode? Or not relevant? (Not a loaded question, genuinely don't know.)
> > 
> > Sorry I wasn't specific. The values seem to be correct in Composition mode. I don't think there is a bug in that case.
> Funny, tst_qwebpage has:
>     //Set selection with negative length
>     inputAttributes << QInputMethodEvent::Attribute(QInputMethodEvent::Selection, 6, -5, QVariant());
>     QInputMethodEvent eventSelection3("",inputAttributes);
>     page->event(&eventSelection3);
>     //ImAnchorPosition
>     variant = page->inputMethodQuery(Qt::ImAnchorPosition);
>     anchorPosition =  variant.toInt();
>     QCOMPARE(anchorPosition, 1);
>     //ImCursorPosition
>     variant = page->inputMethodQuery(Qt::ImCursorPosition);
>     cursorPosition =  variant.toInt();
>     QCOMPARE(cursorPosition, 6);
> which would suggest the opposite to me, but I don't know too much about input methods.

This test case causes the Editor to go into composistion mode and therefore QWebPage::inputMethodQuery() returns the correct values. I don't know whether it is supposed to go into composition mode in this case or not (And that is the concern I raised in an email to you last week since this test case was part of your commit #58218). The test case I am fixing is when the editor is not in composition mode and is different than this.
Comment 9 Simon Hausmann 2010-05-18 13:58:17 PDT
Comment on attachment 56206 [details]
Prosposed patch to fix the return values for Qt::ImCursorPosition and Qt::ImAnchorPosition in QWebPage::inputMethodQuery() when the Editor doesn't have a composition.

r=me
Comment 10 WebKit Commit Bot 2010-05-20 06:57:12 PDT
Comment on attachment 56206 [details]
Prosposed patch to fix the return values for Qt::ImCursorPosition and Qt::ImAnchorPosition in QWebPage::inputMethodQuery() when the Editor doesn't have a composition.

Clearing flags on attachment: 56206

Committed r59833: <http://trac.webkit.org/changeset/59833>
Comment 11 WebKit Commit Bot 2010-05-20 06:57:20 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Simon Hausmann 2010-05-20 07:59:28 PDT
Revision r59833 cherry-picked into qtwebkit-2.0 with commit cf6ec10122bf813a16705f55b29f522a78ce83de