MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into account.

MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input. Also, a test is added which fails on the current master.

BUG=624657
Committed: https://crrev.com/89e44b6df97a0f5c63586de6a9716b4e18339c23
Cr-Commit-Position: refs/heads/master@{#408331}

[karandeepb, 2016-07-26 03:40:51]

karandeepb@chromium.org changed reviewers:
+ tapted@chromium.org

[karandeepb, 2016-07-26 03:40:52]

PTAL Trent.

https://codereview.chromium.org/2177853002/diff/1/ui/views/cocoa/bridged_cont...
File ui/views/cocoa/bridged_content_view.mm (right):

https://codereview.chromium.org/2177853002/diff/1/ui/views/cocoa/bridged_cont...
ui/views/cocoa/bridged_content_view.mm:1225:
textInputClient_->DeleteRange(gfx::Range(replacementRange));
The only case I have found where a valid replacementRange is being passed is
when accented characters are input. Do you know of any other possible case?
Asking because apple docs for the method say  "replacementRange:The range to
replace, computed from the beginning of the marked text." which seems slightly
incorrect, since replacementRange seems to be computed from the beginning of the
text not only marked/composed text.

[tapted, 2016-07-26 05:40:27]

Can we update BridgedNativeWidgetTest.TextInput_Compose with a case for a
non-empty replacement range?

https://codereview.chromium.org/2177853002/diff/1/ui/views/cocoa/bridged_cont...
File ui/views/cocoa/bridged_content_view.mm (right):

https://codereview.chromium.org/2177853002/diff/1/ui/views/cocoa/bridged_cont...
ui/views/cocoa/bridged_content_view.mm:1225:
textInputClient_->DeleteRange(gfx::Range(replacementRange));
On 2016/07/26 03:40:52, karandeepb wrote:
> The only case I have found where a valid replacementRange is being passed is
> when accented characters are input. Do you know of any other possible case?
> Asking because apple docs for the method say  "replacementRange:The range to
> replace, computed from the beginning of the marked text." which seems slightly
> incorrect, since replacementRange seems to be computed from the beginning of
the
> text not only marked/composed text.

I don't really know IME code paths TBH. One guess: maybe when tabbing through
choices in the IME input window?

[karandeepb, 2016-07-27 01:51:25]

Description was changed from

==========
MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into
account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input.

BUG=624657
==========

to

==========
MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into
account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input. Also, a test is added which fails on the current master.

BUG=624657
==========

[karandeepb, 2016-07-27 01:54:43]

The CQ bit was checked by karandeepb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 01:55:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 02:34:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 02:34:25]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2016-07-27 02:40:09]

Patchset #2 (id:20001) has been deleted

[karandeepb, 2016-07-27 02:45:39]

On 2016/07/26 05:40:27, tapted wrote:
> Can we update BridgedNativeWidgetTest.TextInput_Compose with a case for a
> non-empty replacement range?
> 
>
https://codereview.chromium.org/2177853002/diff/1/ui/views/cocoa/bridged_cont...
> File ui/views/cocoa/bridged_content_view.mm (right):
> 
>
https://codereview.chromium.org/2177853002/diff/1/ui/views/cocoa/bridged_cont...
> ui/views/cocoa/bridged_content_view.mm:1225:
> textInputClient_->DeleteRange(gfx::Range(replacementRange));
> On 2016/07/26 03:40:52, karandeepb wrote:
> > The only case I have found where a valid replacementRange is being passed is
> > when accented characters are input. Do you know of any other possible case?
> > Asking because apple docs for the method say  "replacementRange:The range to
> > replace, computed from the beginning of the marked text." which seems
slightly
> > incorrect, since replacementRange seems to be computed from the beginning of
> the
> > text not only marked/composed text.
> 
> I don't really know IME code paths TBH. One guess: maybe when tabbing through
> choices in the IME input window?

PTAL Trent. Have added a new test. Also, the diff from patch 1 may be confusing
(since I changed the patch set dependency order), so see the total diff.

[tapted, 2016-07-27 03:06:20]

lgtm

[karandeepb, 2016-07-28 03:03:04]

The CQ bit was checked by karandeepb@chromium.org

[commit-bot: I haz the power, 2016-07-28 03:03:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 03:48:10]

Description was changed from

==========
MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into
account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input. Also, a test is added which fails on the current master.

BUG=624657
==========

to

==========
MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into
account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input. Also, a test is added which fails on the current master.

BUG=624657
==========

[commit-bot: I haz the power, 2016-07-28 03:48:12]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-07-28 03:49:52]

Description was changed from

==========
MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into
account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input. Also, a test is added which fails on the current master.

BUG=624657
==========

to

==========
MacViews: Fix setMarkedText in BridgedContentView to take replacementRange into
account.

This CL changes setMarkedText:selectedRange:replacementRange: method implemented
by BridgedContentView to take replacementRange parameter into account. This
prevents the insertion of redudant characters by the IME, when accented
characters are input. Also, a test is added which fails on the current master.

BUG=624657

Committed: https://crrev.com/89e44b6df97a0f5c63586de6a9716b4e18339c23
Cr-Commit-Position: refs/heads/master@{#408331}
==========

[commit-bot: I haz the power, 2016-07-28 03:49:53]

Patchset 2 (id:??) landed as
https://crrev.com/89e44b6df97a0f5c63586de6a9716b4e18339c23
Cr-Commit-Position: refs/heads/master@{#408331}