New issue
Advanced search Search tips

Issue 895728 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 845079



Sign in to add a comment

KeyboardShowOverride is not restored when ARC++ IME is uninstalled not deactivated.

Project Member Reported by yhanada@chromium.org, Oct 16

Issue description

We disable Chrome OS VK while ARC++ IME by overriding KeyboardShowOverride configuration in keyboard_util.h.
input_method::InputMethodEngineBase::On{Activate,Deactivate} is used to track enabling/disabling ARC++ IME, but they are not called when ARC++ IME in uninstalled.
We should use OnActiveImeChanged() instead of them to track it correctly.

 
Note that it's critical because Chrome OS VK can be disabled forever by this bug.

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 16

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

commit dd3006ca39d0f50feb4e687488f4b8c9653473cd
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Tue Oct 16 17:34:09 2018

Restore KeyboardShowOverride when the ARC IME is uninstalled.

We disable Chrome OS VK while ARC++ IME by observing
OnActivate/OnDeactivate event, but it's not called when the IME is
removed.
This CL moves the logic to disable/re-enable Chrome OS VK to
InputMethodChanged which is called always when the active IME is
changed.

Bug:  895728 
Test: manual & unit_tests
Change-Id: If57a725de23ae3315046bc4f511ac9d343bfbf8d
Reviewed-on: https://chromium-review.googlesource.com/c/1282690
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600028}
[modify] https://crrev.com/dd3006ca39d0f50feb4e687488f4b8c9653473cd/chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.cc
[modify] https://crrev.com/dd3006ca39d0f50feb4e687488f4b8c9653473cd/chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service_unittest.cc
[modify] https://crrev.com/dd3006ca39d0f50feb4e687488f4b8c9653473cd/components/arc/ime/arc_ime_service.cc

Labels: Merge-Rejected-71
requesting merge to M-71
Labels: -Merge-Rejected-71 Merge-Request-71
oops, I set the wrong label :/
Please provide details on testing / verification.  Needed prior to merge approval.  Thanks.

Repro steps is:
0. Put the device in tablet mode
1. Enable arc-input-method flag in chrome://flags
2. Install Gboard from PlayStore
3. Enable it from chrome://settings
4. Change active IME to Gboard
5. Uninstall Gboard from PlayStore

Expected result:
Automatically switch to Chrome OS IME and VK shows up.

Actual result:
Automatically switch to Chrome OS IME, but VK doesn't show up.

So, the fix can be verified by following the repro steps and checking VK of Chrome OS is still working.
Thanks!
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 25

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Thanks for #6; was the fix verified?
I verified it on latest canary. Should I ask someone else to verify it?
Labels: -Merge-Review-71 Merge-Approved-71
No, as long as it was verified..   Thanks

Approving merge to M71 Chrome OS.

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 29

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a68bca55be153ed74cd4ec86631274f8e08860a

commit 2a68bca55be153ed74cd4ec86631274f8e08860a
Author: Yuichiro Hanada <yhanada@chromium.org>
Date: Mon Oct 29 08:20:29 2018

Restore KeyboardShowOverride when the ARC IME is uninstalled.

We disable Chrome OS VK while ARC++ IME by observing
OnActivate/OnDeactivate event, but it's not called when the IME is
removed.
This CL moves the logic to disable/re-enable Chrome OS VK to
InputMethodChanged which is called always when the active IME is
changed.

Bug:  895728 
Test: manual & unit_tests
Change-Id: If57a725de23ae3315046bc4f511ac9d343bfbf8d
Reviewed-on: https://chromium-review.googlesource.com/c/1282690
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600028}(cherry picked from commit dd3006ca39d0f50feb4e687488f4b8c9653473cd)
Reviewed-on: https://chromium-review.googlesource.com/c/1303323
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#365}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/2a68bca55be153ed74cd4ec86631274f8e08860a/chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service.cc
[modify] https://crrev.com/2a68bca55be153ed74cd4ec86631274f8e08860a/chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service_unittest.cc
[modify] https://crrev.com/2a68bca55be153ed74cd4ec86631274f8e08860a/components/arc/ime/arc_ime_service.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/2a68bca55be153ed74cd4ec86631274f8e08860a

Commit: 2a68bca55be153ed74cd4ec86631274f8e08860a
Author: yhanada@chromium.org
Commiter: yhanada@chromium.org
Date: 2018-10-29 08:20:29 +0000 UTC

Restore KeyboardShowOverride when the ARC IME is uninstalled.

We disable Chrome OS VK while ARC++ IME by observing
OnActivate/OnDeactivate event, but it's not called when the IME is
removed.
This CL moves the logic to disable/re-enable Chrome OS VK to
InputMethodChanged which is called always when the active IME is
changed.

Bug:  895728 
Test: manual & unit_tests
Change-Id: If57a725de23ae3315046bc4f511ac9d343bfbf8d
Reviewed-on: https://chromium-review.googlesource.com/c/1282690
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600028}(cherry picked from commit dd3006ca39d0f50feb4e687488f4b8c9653473cd)
Reviewed-on: https://chromium-review.googlesource.com/c/1303323
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#365}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)
Thank you for reviewing my request!

Sign in to add a comment