New issue
Advanced search Search tips

Issue 901439 link

Starred by 1 user
Status: Fixed
Owner:
yhanada@chromium.org
Closed: Yesterday
Cc:
shuchen@chromium.org
sadrul@chromium.org
tetsui@chromium.org
yusukes@chromium.org
ekaramad@chromium.org
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 1
Type: Bug

Blocked on:
issue 908762


Sign in to add a comment

Value of RWHVA::Get{TextFromRange, SelectionRange, TextRange} are not updated sometimes.

Project Member Reported by yhanada@chromium.org, Nov 2 
Spun off from crbug.com/897691
This issue is for all aura platforms.

RWHVA is using TextInputManager::GetTextSelection() to answer to these calls. Results of TextInputManager::GetTextSelection() are updated once TextInputManager::SelectionChanged(), but there are some cases it's not called even when the text is updated.
For example, calling RWHVA::SetCompositionText and ExtendSelectionAndDelete updates the text, but SelectionChanged() is never called. Therefore, the result of GetTextFromRange() is never updated.

Comment 1 by yhanada@chromium.org, Nov 27

Cc: ekaramad@chromium.org shuchen@chromium.org tetsui@chromium.org
Components: Platform>Apps>ARC UI>Input>Text>IME Blink>Editing>Selection
Status: Started (was: Assigned)
As per discussion in crrev.com/c/1310894, I noticed GetSelectionRange is used for getting selection in non-editable text (at least TouchSelectionControllerClientAura uses it).
We need two APIs to get selection only in editable text field and selection in any text view. The latter function is already provided by RenderWidgetHostViewBase::GetSelectedText(), so I'm going to do:
1. Make users of RWHVA::GetSelectionRange use GetSelectedText if they need selection in non-editable text
2. Make RWHVA::GetSelectionRange return only selections in editable text field by using TextInputState

Comment 2 by yhanada@chromium.org, Nov 27

Blockedon: 908762
Project Member

Comment 4 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 5 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 6 by bugdroid1@chromium.org, Yesterday (28 hours ago) 

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

commit 08ea0fccd8ccbbf61dcf7b93a857dd1b6844b3d0
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Tue Jan 22 03:19:51 2019

Use TextInputState to provide state of a text field.

RWHVA::{GetTextRange, GetTextFromRange} used
TextInputManager::TextSelection provided by TextInputManager which
includes texts outside of a text field.
This CL changes content getter methods of TextInputClient
interface to use TextInputState to avoid inconsistency caused by using
results of different IPCs.

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

Comment 7 by yhanada@chromium.org, Yesterday (28 hours ago)

Status: Fixed (was: Started)
►

Sign in to add a comment