Bug 35933 - [Qt] [Symbian] Can not backward select (highlight) text using virtual keyboard
Summary: [Qt] [Symbian] Can not backward select (highlight) text using virtual keyboard
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: S60 Hardware S60 3rd edition
: P2 Normal
Assignee: Shen Yi
URL:
Keywords: PlatformOnly, Qt
Depends on:
Blocks:
 
Reported: 2010-03-09 11:20 PST by Shen Yi
Modified: 2010-03-29 04:02 PDT (History)
4 users (show)

See Also:


Attachments
proposal fix for bug 35933 (783 bytes, patch)
2010-03-09 11:37 PST, Shen Yi
no flags Details | Formatted Diff | Diff
proposal patch (658 bytes, patch)
2010-03-16 08:20 PDT, Shen Yi
no flags Details | Formatted Diff | Diff
patch for bug35933 (1.38 KB, patch)
2010-03-16 11:20 PDT, Shen Yi
hausmann: review-
Details | Formatted Diff | Diff
new patch with the unit test (2.69 KB, patch)
2010-03-19 08:54 PDT, Shen Yi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shen Yi 2010-03-09 11:20:37 PST
By using the virtual keyboard on a symbian device, the user can not select texts from right to left (the cursor position is before the anchor position);
Comment 1 Shen Yi 2010-03-09 11:37:45 PST
Created attachment 50334 [details]
proposal fix for bug 35933
Comment 2 Shen Yi 2010-03-09 11:55:19 PST
This issue is Qt only, and the root cause is that webcore assumes the selection's start position is always before the end position.

In RenderTextControl.cpp;

void RenderTextControl::setSelectionEnd(int end)
{
    setSelectionRange(min(end, selectionStart()), end);
}

The setSelectionEnd(int) function was called from QWebPagePrivate::inputMethodEvent(QInputMethodEvent*) at qwebpage.cpp

  case QInputMethodEvent::Selection: {
     if (renderTextControl) {
         renderTextControl->setSelectionStart(a.start);
         renderTextControl->setSelectionEnd(a.start + a.length);
     }
     break;
  }

However, in above code, the a.length, which is from QcoeFepInputContext, is negative when doing backward selection:

void QCoeFepInputContext::SetCursorSelectionForFepL(const TCursorSelection&) {
    int pos = aCursorSelection.iAnchorPos;
    int length = aCursorSelection.iCursorPos - pos;
    QList<QInputMethodEvent::Attribute> attributes;
    attributes << QInputMethodEvent::Attribute(QInputMethodEvent::Selection, pos, length, QVariant());
}

It can explain why the backward selection doesn't work, that is,

when start position is larger than end position,

setSelectionRange(min(end, selectionStart()), end);
  equals
setSelectionRange(end, end);

The patch I attached would check/adjust the start & end position before calling the setSelectionStart & setSelectionEnd. I have tested it on both s60 emulator & target.
Comment 3 Shen Yi 2010-03-16 08:20:20 PDT
Created attachment 50795 [details]
proposal patch
Comment 4 Shen Yi 2010-03-16 11:20:07 PDT
Created attachment 50812 [details]
patch for bug35933
Comment 5 Simon Hausmann 2010-03-19 00:57:27 PDT
Comment on attachment 50812 [details]
patch for bug35933

I think the patch is fine, but please make a unit test for this. See the inputMethods() unit tests in WebKit/qt/tests/qwebpage.

You could test this by sending a synthetic input method event with a selection set. After sending the event you could verify that the selection is set correctly.
Comment 6 Shen Yi 2010-03-19 08:54:21 PDT
Created attachment 51157 [details]
new patch with the unit test

The new patch contains the unit test for qwebpage.
Comment 7 WebKit Commit Bot 2010-03-22 08:10:04 PDT
Comment on attachment 51157 [details]
new patch with the unit test

Clearing flags on attachment: 51157

Committed r56334: <http://trac.webkit.org/changeset/56334>
Comment 8 WebKit Commit Bot 2010-03-22 08:10:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Hausmann 2010-03-29 04:02:58 PDT
Cherry-picked into qtwebkit-4.6 with commit edc778e0b452640d8e05c4a88903c18b6afe20ec