Mac: Fix Korean input on OSX 10.11

Mac: Fix Korean input on OSX 10.11

handling_input_event_ needs to be set in RenderWidget::OnImeSetComposition,
otherwise RenderView::DidChangeSelection will skip sending the selection
change message the the browser, even though it was correctly set on the
renderer side.

Also fix incorrectly set selectedRange_ in -setMarkedText,
|newSelRange| is relative to the marked text while selectedRange_ is
relative to the entire text storage.

BUG=500376
Committed: https://crrev.com/28025f2c0eebbd850c745005f955b53b9dfa1cdf
Cr-Commit-Position: refs/heads/master@{#339754}

[Andre, 2015-07-16 22:36:46]

andresantoso@chromium.org changed reviewers:
+ suzhe@chromium.org

[Andre, 2015-07-16 22:36:47]

James, PTAL.
 I'm very unfamiliar with this code so I'm not sure what the proper fix is.
From my debugging, the problem seems to be that the value we return from
-selectedRange does not include the currently marked text.
Also, I don't think the setting of selectedRange_ in -setMarkedText is correct,
|newSelRange| is relative to the marked text while selectedRange_ should be
relative to the entire text storage.

[James Su, 2015-07-17 03:47:37]

Your change makes sense, except one concern.

https://codereview.chromium.org/1240943002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/1240943002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_mac.mm:2869: return
NSMakeRange(NSMaxRange(markedRange_), 0);
I'm not sure if this logic is necessary. The developer document:
https://developer.apple.com/library/mac/documentation/Cocoa/Reference/NSTextI...
doesn't mention this behavior.

Can you remove this whole method and test the behavior?

[Andre, 2015-07-17 18:05:44]

https://codereview.chromium.org/1240943002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/1240943002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_mac.mm:2869: return
NSMakeRange(NSMaxRange(markedRange_), 0);
Thanks James.
This is actually the main fix for the bug.
AppKit calls this then calls -attributedSubstringForProposedRange:actualRange:
based on the returned value.
It ends up asking for the wrong substring for working with the current
composition because selectedRange is
wrong.
The selection caret is placed after the marked text, so selectedRange should
point the end of the text,
but the selectedRange we get from blink does not include the marked text for
some reason.
For example, if we have 2 commited characters and 1 character marked text,
selectedRange should be (3, 0)
but we were returning (2,0).

[James Su, 2015-07-18 02:26:33]

https://codereview.chromium.org/1240943002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/1240943002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_mac.mm:2869: return
NSMakeRange(NSMaxRange(markedRange_), 0);
On 2015/07/17 18:05:44, Andre wrote:
> Thanks James.
> This is actually the main fix for the bug.
> AppKit calls this then calls -attributedSubstringForProposedRange:actualRange:
> based on the returned value.
> It ends up asking for the wrong substring for working with the current
> composition because selectedRange is
> wrong.
> The selection caret is placed after the marked text, so selectedRange should
> point the end of the text,
> but the selectedRange we get from blink does not include the marked text for
> some reason.
> For example, if we have 2 commited characters and 1 character marked text,
> selectedRange should be (3, 0)
> but we were returning (2,0).

Can you investigate the root cause of this issue? I'd prefer to fix the root
cause rather than workaround the issue here.

[Andre, 2015-07-20 20:27:32]

Thanks. I traced blink's composition and selection code, and I think the problem
is with
handling_input_event_ not being set to true in
RenderWidget::OnImeSetComposition.
This caused RenderView::DidChangeSelection to skip sending the selection change
message the the browser, even though it was correctly set on the renderer side.
PTAL at patch 2.

[James Su, 2015-07-21 01:52:24]

lgtm

[Andre, 2015-07-21 18:14:24]

andresantoso@chromium.org changed reviewers:
+ avi@chromium.org

[Andre, 2015-07-21 18:14:24]

Avi PTAL for owners.

[Avi (use Gerrit), 2015-07-21 20:49:08]

lgtm

stampity stamp

[Andre, 2015-07-21 21:02:25]

The CQ bit was checked by andresantoso@chromium.org

[Andre, 2015-07-21 21:02:26]

The patchset sent to the CQ was uploaded after l-g-t-m from suzhe@chromium.org
Link to the patchset: https://codereview.chromium.org/1240943002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2015-07-21 21:04:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240943002/40001

[commit-bot: I haz the power, 2015-07-21 21:46:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-21 21:47:14]

Patchset 3 (id:??) landed as
https://crrev.com/28025f2c0eebbd850c745005f955b53b9dfa1cdf
Cr-Commit-Position: refs/heads/master@{#339754}