ui/wm/core/ime_util_chromeos doesn't work in mash |
|||
Issue descriptionThese functions directly manipulate the bounds of windows, which doesn't do the same thing in client code. This code needs to be updated for mash. Evan, any chance you could take a look? I suspect this code needs to be operating at the widget level, but I haven't looked deeply.
,
Dec 6
I did some quick testing and wm::EnsureWindowNotInRect is not getting called at all in SPM. Investigating further.
,
Dec 6
The problem appears to be that this call in keyboard_controller.cc: ime->GetTextInputClient()->EnsureCaretNotInRect(occluded_bounds); Is getting a RemoteTextInputClient instance, which has EnsureCaretNotInRect as NOTIMPLEMENTED: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/ime_driver/remote_text_input_client.cc?type=cs&sq=package:chromium&g=0&l=166 I think that the correct fix may be to call ime->GetTextInputClient()->EnsureCaretNotInRect(occluded_bounds) on the Chrome side. We already have a Mojo based observer for the occluded bounds changin. I will look into that as part of issue 906859 . (i.e. this may not actually be related to that).
,
Dec 6
+xiyuan
,
Dec 6
Let me wire up RemoteTextInputClient::EnsureCaretNotInRect to call into the real TextInputClient. How to test this?
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ffb4d235d408e6e06f749e65406c2e652e9fa10c commit ffb4d235d408e6e06f749e65406c2e652e9fa10c Author: Xiyuan Xia <xiyuan@chromium.org> Date: Mon Dec 10 17:47:06 2018 SingleProcessMash: Fix browser window move with VK - Wire up RemoteTextInputClient::EnsureCaretNotInRect to forward the call to the actual TextInputClient; - Fix ime_util_chromeos to handle windows under MUS mode; - Change ChromeKeyboardBoundsObserver to use WidgetObserver to get bounds change of toplevel browser window under mash; Bug: 631527, 906888 , 906859 Change-Id: I1570f8e3254e025106a120cf5ee538c20367c405 Reviewed-on: https://chromium-review.googlesource.com/c/1366879 Commit-Queue: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#615164} [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/ash/keyboard/chrome_keyboard_bounds_observer.cc [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/ash/keyboard/chrome_keyboard_bounds_observer.h [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/views/ime_driver/remote_text_input_client.cc [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/services/ws/ime/ime_unittest.cc [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/services/ws/public/mojom/ime/ime.mojom [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/aura/mus/text_input_client_impl.cc [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/aura/mus/text_input_client_impl.h [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/views/widget/desktop_aura/desktop_screen_position_client.cc [modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/wm/core/ime_util_chromeos.cc
,
Dec 10
CL above makes ime_util_chromeos to use aura::Window::SetBoundsInScreen to move the window. Ideally we want to use the widget-level API but //content (RWHVAura) depends on the code and does not depend on //ui/views.
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a9470f46cef9edc682924a80264914bf404c3e70 commit a9470f46cef9edc682924a80264914bf404c3e70 Author: Steven Bennetts <stevenjb@chromium.org> Date: Wed Dec 12 01:37:15 2018 Clean up ChromeKeyboardBoundsObserver and fix for Mash https://crrev.com/c/1366879 fixed small window repositioning for Mash but exposed a bug in ChromeKeyboardBoundsObserver::ShouldEnableInsets. This fixes that issue and includes some cleanup done while debugging. It also updates the filter file tpo remove tests that are now passing! Bug: 906888 Change-Id: I64dcfe8de4a5970d69a76f26646b80be01016a50 Reviewed-on: https://chromium-review.googlesource.com/c/1372319 Commit-Queue: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Cr-Commit-Position: refs/heads/master@{#615752} [modify] https://crrev.com/a9470f46cef9edc682924a80264914bf404c3e70/ash/keyboard/ash_keyboard_ui.cc [modify] https://crrev.com/a9470f46cef9edc682924a80264914bf404c3e70/chrome/browser/ui/ash/keyboard/chrome_keyboard_bounds_observer.cc [modify] https://crrev.com/a9470f46cef9edc682924a80264914bf404c3e70/chrome/browser/ui/ash/keyboard/chrome_keyboard_bounds_observer.h [modify] https://crrev.com/a9470f46cef9edc682924a80264914bf404c3e70/chrome/browser/ui/ash/keyboard/keyboard_end_to_end_browsertest.cc [modify] https://crrev.com/a9470f46cef9edc682924a80264914bf404c3e70/testing/buildbot/filters/chromeos.single_process_mash.browser_tests.filter |
|||
►
Sign in to add a comment |
|||
Comment 1 by est...@chromium.org
, Nov 20