Mac: Let arrow keys move caret in IME mode.

Mac: Let arrow keys move caret in IME mode.

BUG=26869
TEST=Activate Kotoeri, smash keys, confirm that arrow keys move caret. Confirm that rest of IME still works (hah!).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=31986

[Nico, 2009-11-13 23:25:01]

Please help review.

Things still seem to work with this patch as far as I can tell (the caret is now
sometimes to the left of the currently active glyph, but that doesn't feel to
weird to me. Safari hides the caret while IME is active). At least moving the
caret while IME is active does now work.

[Nico, 2009-11-14 01:09:36]

Played some more with this.

With the following patch, our behavior seems to match Safari's:

Index: third_party/WebKit/WebKit/chromium/src/WebViewImpl.cpp
===================================================================
--- third_party/WebKit/WebKit/chromium/src/WebViewImpl.cpp	(revision 50932)
+++ third_party/WebKit/WebKit/chromium/src/WebViewImpl.cpp	(working copy)
@@ -1026,7 +1026,7 @@
         // Therefore, we use the 'cursorPosition' for these arguments so that
         // there are not any characters in the above region.
         editor->setComposition(compositionString, underlines,
-                               cursorPosition, cursorPosition);
+                               cursorPosition, targetEnd);
         // The given string is a result string, which means the ongoing
         // composition has been completed. I have to call the
         // Editor::confirmCompletion() and complete this composition.


However, that has implications for the other platforms as well…will send a mail
to chromium-dev and ask for opinions I guess.

[dmac, 2009-11-14 01:18:58]

LGTM with minor nit

http://codereview.chromium.org/386032/diff/6001/7001
File chrome/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/386032/diff/6001/7001#newcode1282
Line 1282: int length = [im_text length];
Do we care about the NSUInteger to int conversions going on here?

http://codereview.chromium.org/386032/diff/6001/7001#newcode1283
Line 1283: int cursor;
init cursor here with newSelRange.location

[tesigoadondequieras_gmail.com, 2009-11-14 01:21:13]

REMOVER
----- Original Message ----- 
From: <dmaclach@chromium.org>
To: <thakis@chromium.org>; <hbono@chromium.org>; <suzhe@chromium.org>; 
<jeremy@chromium.org>; <avi@chromium.org>
Cc: <chromium-reviews@googlegroups.com>
Sent: Friday, November 13, 2009 11:18 PM
Subject: [chromium-reviews] Re: Mac: Let arrow keys move caret in IME mode.


> LGTM with minor nit
>
>
> http://codereview.chromium.org/386032/diff/6001/7001
> File chrome/browser/renderer_host/render_widget_host_view_mac.mm
> (right):
>
> http://codereview.chromium.org/386032/diff/6001/7001#newcode1282
> Line 1282: int length = [im_text length];
> Do we care about the NSUInteger to int conversions going on here?
>
> http://codereview.chromium.org/386032/diff/6001/7001#newcode1283
> Line 1283: int cursor;
> init cursor here with newSelRange.location
>
> http://codereview.chromium.org/386032 

[Nico, 2009-11-14 01:22:47]

I'll wait a day or so with committing this.

http://codereview.chromium.org/386032/diff/6001/7001
File chrome/browser/renderer_host/render_widget_host_view_mac.mm (right):

http://codereview.chromium.org/386032/diff/6001/7001#newcode1282
Line 1282: int length = [im_text length];
On 2009/11/14 01:18:58, dmac wrote:
> Do we care about the NSUInteger to int conversions going on here?

No. I doubt people will compose a string with > 2MM characters in a single IME
interaction.

http://codereview.chromium.org/386032/diff/6001/7001#newcode1283
Line 1283: int cursor;
On 2009/11/14 01:18:58, dmac wrote:
> init cursor here with newSelRange.location

Done.

[Nico, 2009-11-14 02:01:53]

On second thought, I'm going to land this, since it's an improvement over the
current situation. I've opened https://bugs.webkit.org/show_bug.cgi?id=31502 for
the WebKit patch; I'd appreciate if people could take a look at that too.