[webkit-reviews] review granted: [Bug 35702] [Qt] Add more support for InputTextController : [Attachment 51824] Updated Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Robert Hogan
<robert at webkit.org>'s request for review:
Bug 35702: [Qt] Add more support for InputTextController
https://bugs.webkit.org/show_bug.cgi?id=35702

Attachment 51824: Updated Patch
https://bugs.webkit.org/attachment.cgi?id=51824&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

> -	       if (renderTextControl) {
> -		   renderTextControl->setSelectionStart(qMin(a.start, (a.start
+ a.length)));
> -		   renderTextControl->setSelectionEnd(qMax(a.start, (a.start +
a.length)));
> -	       }
> +	       selection = a;
> +	       hasSelection = true;
>	       break;
>	   }
>  #endif
> @@ -1334,10 +1382,23 @@ void
QWebPagePrivate::inputMethodEvent(QInputMethodEvent *ev)

> +#if QT_VERSION >= 0x040600
> +	   if (hasSelection) {
> +	       QString text = (renderTextControl) ?
QString(renderTextControl->text()) : QString();
> +	       if (preedit.isEmpty() && selection.start + selection.length > 0)

> +		   preedit = text;
> +	       editor->setComposition(preedit, underlines, selection.start,
selection.start + selection.length);
> +	   } else
> +#endif
> +	       editor->setComposition(preedit, underlines, preedit.length(),
0);

Looks good to me, but before landing please verify that the above two hunks
don't break the auto-test added in http://trac.webkit.org/changeset/56334 ,
which verifies that selection start/end are swapped correctly if they are
reversed (see the qMin/qMax usage).