New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 640816 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

arc: Stop piggybacking "text input started" and "show virtual keyboard" event

Project Member Reported by kinaba@chromium.org, Aug 24 2016

Issue description

Internal bug: b/31001453.

Let's remove the following if-block marked as TODO.

https://chromium.googlesource.com/chromium/src/+/master/components/arc/ime/arc_ime_service.cc#145

    input_method->OnTextInputTypeChanged(this);
    // TODO(crbug.com/581282): Remove this piggyback call when
    // ImeInstance::ShowImeIfNeeded is wired to ARC.
    if (input_method->GetTextInputClient() == this &&
        ime_type_ != ui::TEXT_INPUT_TYPE_NONE) {
      input_method->ShowImeIfNeeded();
    }

On Android IMF these two are conceptually distinct, and some apps like Hangouts
takes this liberty. The above code is not just unnecessary now but also
being an obstacle to make such apps work better b/31001453.
 

Comment 1 by kinaba@chromium.org, Aug 24 2016

Labels: OS-Chrome
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 25 2016

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

commit 17a7a0c83e3615a6f8ad9d56e4b82006b53660ff
Author: kinaba <kinaba@chromium.org>
Date: Thu Aug 25 03:11:59 2016

arc: Stop piggybacking "text input started" and "show virtual keyboard" events.

It is not only useless in the latest version, but is even harmful
for some advanced apps that take the liberty of splitting these
two events.

This change itself should not change any observable behavior yet,
but will be necessary to enable coming Android-side change to
make this split effective.

BUG= 640816 
BUG=b/31001453
TEST=ArcImeServiceTest
TEST=Tested manually with some ARC apps (Settings, PlayStore)

Review-Url: https://codereview.chromium.org/2280503002
Cr-Commit-Position: refs/heads/master@{#414283}

[modify] https://crrev.com/17a7a0c83e3615a6f8ad9d56e4b82006b53660ff/components/arc/ime/arc_ime_service.cc
[modify] https://crrev.com/17a7a0c83e3615a6f8ad9d56e4b82006b53660ff/components/arc/ime/arc_ime_service_unittest.cc

Comment 3 by kinaba@chromium.org, Aug 26 2016

Labels: Merge-Request-53
Sorry for the last minute merge-request, but I hope the change is
* very same to merge: it only affects ARC apps in Chrome OS in tablet mode.
* has an impact on some popular ARC apps (notably Hangouts)

Comment 4 by dimu@chromium.org, Aug 26 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M53, manual review required.

Comment 5 by kinaba@chromium.org, Aug 31 2016

Labels: -M-53 -Hotlist-Merge-review -Merge-Review-53 M-54
Status: Fixed (was: Started)
Internal bug punted to M54. Closing

Comment 6 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 7 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 8 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 9 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 10 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment