New issue
Advanced search Search tips

Issue 908762 link

Starred by 1 user
Status: Fixed
Owner:
yhanada@chromium.org
Closed: Jan 10
Cc:
shuchen@chromium.org
sadrul@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug

Blocking:
issue 901439



Sign in to add a comment

Make RWHVA::GetSelectionRange return only selections in editable text field

Project Member Reported by yhanada@chromium.org, Nov 27 
Please also see  crbug.com/901439 

GetSelectionRange (declared in TextInputClient) is intended to use by IMEs, so implementations of the method except RWHVA::GetSelectionRange returns the range of selections only in editable field.

Currently some class depends on it that RWHVA::GetSelectionRange reruens selections in non-editable text.
Let's make them use RWHVA::GetSelectedText() instead of GetSelectionRange.

Comment 1 by sadrul@chromium.org, Nov 28 

I suggest renaming GetSelectionRange() to something that makes it clear that this is only for editable text (E.g. GetEditableSelectionRange() or something like that).

Comment 3 by yhanada@chromium.org, Dec 12

Components: UI>Input>Text>IME
I'm working on renaming {Get, Set}SelectionRange to {Get, Set}EditableSelectionRange respectively.

Comment 4 by yhanada@chromium.org, Dec 12

Labels: -M-72 M-73
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 13 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2dc2026c2fbcb2345b92b5a5a36dfa03c507d66c

commit 2dc2026c2fbcb2345b92b5a5a36dfa03c507d66c
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Thu Dec 13 01:28:34 2018

Use RWHVA::GetSelectedText() to get selected text in RWHVA.

GetSelectionRange() and GetTextFromRange() will be changed to
return only selected text in editable text field.
To avoid breaking current behavior, this CL switches them to
GetSelectedText() which will continue to return all selected texts.

Bug:  901439 ,  908762 
Test: content_browsertests
Change-Id: I9fbd5b8ba887fc52ac75e625908b754ebaa47426
Reviewed-on: https://chromium-review.googlesource.com/c/1373732
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616149}
[modify] https://crrev.com/2dc2026c2fbcb2345b92b5a5a36dfa03c507d66c/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/65eaaf4ff757a827e5477b123fd772d7da49b78a

commit 65eaaf4ff757a827e5477b123fd772d7da49b78a
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Fri Dec 14 07:18:38 2018

Rename {Get,Set}SelectionRange to {Get,Set}EditableSelectionRange.

Some implementation of TextInputClient interface, for example RWHVA, can
have text outside of an editable text field.
For those views, {Get, Set}SelectionRange is confusing names because
it's not clear whether they return selections outside of the editable
text field or not.
This CL renames them to {Get, Set}EditableSelectionRange to make it
clear that they returns only selections in an editable text field.

Bug:  901439 ,  908762 
Test: Build.
Change-Id: I99e6140a635a61b755ebfa35efff2d306cec8d32
Reviewed-on: https://chromium-review.googlesource.com/c/1373932
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Mitsuru Oshima (gardener - slow) <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616605}
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/chrome/browser/chromeos/arc/input_method_manager/input_connection_impl.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/chrome/browser/chromeos/input_method/textinput_test_helper.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/chrome/browser/ui/views/ime_driver/remote_text_input_client.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/chrome/browser/ui/views/ime_driver/remote_text_input_client.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/components/arc/ime/arc_ime_service.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/components/arc/ime/arc_ime_service.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/components/arc/ime/arc_ime_service_unittest.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/components/exo/text_input.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/components/exo/text_input.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/dummy_text_input_client.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/dummy_text_input_client.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/input_method_auralinux.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/input_method_auralinux_unittest.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/input_method_base.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/input_method_chromeos.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/input_method_chromeos_unittest.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/input_method_win_base.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/text_input_client.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/base/ime/win/tsf_text_store_unittest.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/cocoa/bridged_native_widget_unittest.mm
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/controls/prefix_selector.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/controls/prefix_selector.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/controls/textfield/textfield.h
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/controls/textfield/textfield_unittest.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views/examples/label_example.cc
[modify] https://crrev.com/65eaaf4ff757a827e5477b123fd772d7da49b78a/ui/views_bridge_mac/bridged_content_view.mm
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02bbec9a59fe423f049dbc5c215f1528864676c6

commit 02bbec9a59fe423f049dbc5c215f1528864676c6
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Thu Jan 10 03:54:44 2019

Change GetEditableSelectionRange to use TextInputState.

All callers of GetEditableSelectionRange expects a selection in editable
text field. However, TextSelection provided by TextInputManager contains
a selection in non-editable text.

Bug:  908762 
Test: content_unittests
Change-Id: Idf8618136a576a00ff2800446dae5b79e23c33c8
Reviewed-on: https://chromium-review.googlesource.com/c/1402307
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621455}
[modify] https://crrev.com/02bbec9a59fe423f049dbc5c215f1528864676c6/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/02bbec9a59fe423f049dbc5c215f1528864676c6/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

Comment 8 by yhanada@chromium.org, Jan 10

Status: Fixed (was: Started)
►

Sign in to add a comment