Issue metadata
Sign in to add a comment
|
CrOS Shortcut Viewer a11y: search box doesn't bring up onscreen keyboard |
||||||||||||||||||||||||
Issue description70.0.3524.2 (Official Build) canary (64-bit) Google_Eve.9584.160.0 Steps to repro: # Enable On-screen keyboard in Settings > Manage Accessibility Features # Launch keyboard shortcuts app using ctrl + alt + / # Click or double click in the search field Expected: on-screen keyboard expands Actual: on-screen keyboard says collapsed Once you manually expand the on-screen keyboard, the input is received
,
Aug 22
Actually, reopening since I'll let the team decide whether or not to close this bug based on the above finding.
,
Aug 22
Yes, please keep any bugs specific to the app open, thanks! (if you see bugs that are app-only, please also use the label "Proj-Mash-KSV")
,
Aug 22
I have a WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/1185736
,
Aug 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d10527cab466ac073da1c572a63b2a34d59015ef commit d10527cab466ac073da1c572a63b2a34d59015ef Author: Mike Wasserman <msw@chromium.org> Date: Thu Aug 23 07:10:54 2018 ws: Add InputMethodMus plumbing for ShowVirtualKeyboardIfEnabled TODO: Get the keyboard to close when closing the KSV window. Bug: 876589 Test: a11y virtual keyboard shows on second KSV textfield click. Change-Id: I57a09acb9cf8b784a9c40763028c4733dd43d474 Reviewed-on: https://chromium-review.googlesource.com/1185736 Commit-Queue: Michael Wasserman <msw@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Cr-Commit-Position: refs/heads/master@{#585418} [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/chrome/browser/chromeos/input_method/input_method_manager_impl.cc [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.h [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/chrome/browser/ui/views/ime_driver/simple_input_method.cc [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/chrome/browser/ui/views/ime_driver/simple_input_method.h [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/services/ui/ime/test_ime_driver/test_ime_driver.cc [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/services/ui/public/interfaces/ime/ime.mojom [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/ui/aura/mus/input_method_mus.cc [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/ui/aura/mus/input_method_mus.h [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/ui/aura/mus/input_method_mus_unittest.cc [modify] https://crrev.com/d10527cab466ac073da1c572a63b2a34d59015ef/ui/base/ime/input_method_base.cc
,
Aug 25
AFAICT, ui::KeyboardController only observes one InputMethod (the one supplied by KeyboardUI): https://cs.chromium.org/chromium/src/ui/keyboard/keyboard_controller.cc?rcl=6bd345017a9cf81abd12c271e6a635a084ed9252&l=198 So, it isn't aware of the InputMethodChromeOS instance created by InputMethodBridge: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc?rcl=75a4d21e24696dfb7ed4934257d8c83a0307d656&l=27 Normally it closes via OnTextInputStateChanged (also triggered on DetachTextInputClient). Here are the good callstacks when closing a browser window (posing task and task execution): #1 0x7f7f79627690 keyboard::KeyboardController::HideKeyboardImplicitlyBySystem() #2 0x7f7f79627c72 keyboard::KeyboardController::OnTextInputStateChanged() #3 0x7f7f7aaf5a85 ui::InputMethodBase::NotifyTextInputStateChanged() #4 0x7f7f7aaf58db ui::InputMethodBase::DetachTextInputClient() #5 0x7f7f791735bf views::Textfield::OnBlur() #6 0x55957631447f OmniboxViewViews::OnBlur() #7 0x7f7f7919897f views::View::Blur() #8 0x7f7f79181159 views::FocusManager::SetFocusedViewWithReason() #9 0x7f7f791a0b83 views::Widget::Close() #1 0x7fe8dd958ed7 keyboard::KeyboardController::HideKeyboard() #2 0x7fe8dd95d42b _ZN4base8internal7InvokerINS0_9BindStateIMN8keyboard18KeyboardControllerEFvNS4_10HideReasonEEJNS_7WeakPtrIS4_EES5_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE #3 0x7fe8e3404e55 base::debug::TaskAnnotator::RunTask() Maybe we should avoid multiple InputMethod instances, and just notify the one InputMethodChromeOS of RemoteTextInputClient changes? Otherwise, I guess KeyboardController will need to track multiple InputMethod instances (for the active/focused window?) I have a naive WIP solution, but it might not handle all corner cases (activating another mojo window with a focused textfield on window close, etc. https://chromium-review.googlesource.com/c/chromium/src/+/1188453
,
Aug 25
CC'ing some other folks that might be interested, I'll be out for two weeks. This would be nice to fix for M-70, but not critical for launching KSV, IMO. Hopefully there aren't any other IME issues with the KSV app codepath...
,
Aug 25
To be clear, comment #5 fixed showing the virtual keyboard when focusing the textfield, the keyboard just doesn't close when closing the window.
,
Aug 25
+shuchen/shend - It seems like this is a conflict between mash/mustash creating more than one InputMethodChromeOS (the "shared" one plus one for the shortcut viewer app) and the VK assuming there is only one. Do we know how InputMethodChromeOS will end up after the IME mojo-ification effort?
,
Aug 27
Xiyuan, could you take this over as Mike is out.
,
Aug 28
,
Sep 5
CL in #11 landed on trunk. VK should close after KSV window close.
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dfb07b25e104013694de5bacd87f73a9692fadfe commit dfb07b25e104013694de5bacd87f73a9692fadfe Author: Xiyuan Xia <xiyuan@chromium.org> Date: Tue Sep 04 21:44:38 2018 cros: Fix VK not closing when KSV closes - Make KeyboardController update its observed IME when it is enabled/activated/shown. There could be multiple IME instances with mojo apps. This allows KeyboardController to follow the active IME; - Change ChromeKeyboardUI to return the active IME from IMEBridge instead of the globally shared IME; Bug: 876589 Change-Id: Ieb4cde7131969da3c9182d6bd5447dbe604440fd Reviewed-on: https://chromium-review.googlesource.com/1194986 Reviewed-by: Shu Chen <shuchen@chromium.org> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org> Cr-Commit-Position: refs/heads/master@{#588653} [modify] https://crrev.com/dfb07b25e104013694de5bacd87f73a9692fadfe/chrome/browser/ui/ash/chrome_keyboard_ui.cc [modify] https://crrev.com/dfb07b25e104013694de5bacd87f73a9692fadfe/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/dfb07b25e104013694de5bacd87f73a9692fadfe/ui/keyboard/keyboard_controller.h
,
Dec 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f427007daca2dd5cbbac4908aaffc93d37b184e commit 0f427007daca2dd5cbbac4908aaffc93d37b184e Author: Mike Wasserman <msw@chromium.org> Date: Wed Dec 26 15:23:26 2018 mash: Fix a11y magnifier following caret bounds in SingleProcessMash and KSV Make IMEBridge support multiple observers (per xiyuan@'s crrev.com/c/1197846) Make MagnificationController observe the active app's IME instance. Bug: 876589 Test: Magnifier (Search-Ctrl-m) follows caret focus in SPM and KSV (Ctrl-Alt-/) Change-Id: Id97210ece4c3ad359cb4b31b45c02f5fb020b8e4 Reviewed-on: https://chromium-review.googlesource.com/c/1387669 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Shu Chen <shuchen@chromium.org> Commit-Queue: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#618927} [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/ash/magnifier/magnification_controller.cc [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/ash/magnifier/magnification_controller.h [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/chrome/browser/extensions/api/input_ime/input_ime_api.cc [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/chrome/browser/extensions/api/input_ime/input_ime_api_nonchromeos.cc [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/ui/base/ime/ime_bridge.cc [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/ui/base/ime/ime_bridge.h [modify] https://crrev.com/0f427007daca2dd5cbbac4908aaffc93d37b184e/ui/base/ime/ime_bridge_observer.h
,
Dec 27
Woops, that CL above should have been marked BUG= 876510 |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by leberly@chromium.org
, Aug 22