Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(99)

Issue 826713002: [ChromeOS] Show autofill popup after keyboard (if any) is shown. (Closed)

Created:
5 years, 12 months ago by please use gerrit instead
Modified:
5 years, 8 months ago
Reviewers:
Shu Chen, yukawa, bshe
CC:
chromium-reviews, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, tfarina, penghuang+watch_chromium.org, nona+watch_chromium.org, James Su, aelias_OOO_until_Jul13, Will Harris, Garrett Casto, Evan Stade, SteveT, bshe, sadrul
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[ChromeOS] Show autofill popup after keyboard (if any) is shown. The renderer shows autofill popups immediately upon click or focus change. If the browser shows an IME (virtual keyboard), then it can zoom and scroll the webpage, which will hide the popup. This behavior causes a "flash" of autofill popup on platforms with IME (ChromeOS and Android). The cross-platform fix is for the renderer to wait until the browser shows the virtual keyboard and all related animations are finished. The cross-platform fix along with Android-specific fix is in: https://codereview.chromium.org/715733002/ This patch is the ChromeOS side of the fix. This patch depends on the cross-platform patch to fix the issue. The fix is to change render_widget_host_view_aura behavior to notify the renderer of two events: 1) Keyboard was requested, but the platform does not support virtual keyboards. 2) Keyboard was requested, but it was already showing. These events indicate that the webpage will not be zoomed or scrolled, and so it is safe to show the autofill popup immediately. The ChromeOS implementation is threefold: 1) Keyboard_controller notifies input_method that virtual keyboard is supported and enabled on this platform: InputMethod::SetSupportsOnScreenKeyboard(bool); 2) Keyboard_controller notifies input_method if it is already showing: InputMethod::OnKeyboardBoundsUnchanged(); 3) An InputMethodObserver object sends a ViewMsg_FocusChangeComplete IPC message to the renderer. BUG=430318

Patch Set 1 : #

Total comments: 2

Patch Set 2 : Address comments. #

Total comments: 4

Patch Set 3 : Rebase on top of Android/crossplatform change. #

Patch Set 4 : Disable autofill popup waiting for keyboard when keyboard has been disabled. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -9 lines) Patch
M ash/magnifier/magnification_controller.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/input_method/textinput_test_helper.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/input_method/textinput_test_helper.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_aura.cc View 1 2 4 chunks +37 lines, -0 lines 0 comments Download
M content/common/view_messages.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/render_widget.cc View 1 2 3 chunks +7 lines, -6 lines 0 comments Download
M ui/base/ime/dummy_input_method.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M ui/base/ime/dummy_input_method.cc View 1 2 3 1 chunk +10 lines, -0 lines 0 comments Download
M ui/base/ime/input_method.h View 1 2 3 3 chunks +14 lines, -2 lines 0 comments Download
M ui/base/ime/input_method_base.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M ui/base/ime/input_method_base.cc View 1 2 3 1 chunk +9 lines, -0 lines 0 comments Download
M ui/base/ime/input_method_base_unittest.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/base/ime/input_method_chromeos.h View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M ui/base/ime/input_method_chromeos.cc View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
M ui/base/ime/input_method_observer.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M ui/base/ime/mock_input_method.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M ui/base/ime/mock_input_method.cc View 1 2 3 1 chunk +10 lines, -0 lines 0 comments Download
M ui/base/ime/remote_input_method_win.cc View 1 1 chunk +10 lines, -0 lines 0 comments Download
M ui/base/ime/remote_input_method_win_unittest.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/keyboard/keyboard_controller.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/keyboard/keyboard_controller.cc View 1 2 3 4 chunks +7 lines, -1 line 0 comments Download
M ui/keyboard/webui/vk_mojo_handler.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M ui/keyboard/webui/vk_mojo_handler.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download
M ui/views/ime/input_method_bridge.cc View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 25 (8 generated)
please use gerrit instead
Yohei, PTAL. Please let me know if you can think of a better design that ...
5 years, 12 months ago (2014-12-24 18:21:15 UTC) #5
Yuki
On 2014/12/24 18:21:15, Rouslan S. (back Jan 2nd) wrote: > Yohei, PTAL. > > Please ...
5 years, 12 months ago (2014-12-25 09:41:07 UTC) #7
please use gerrit instead
On 2014/12/25 09:41:07, Yuki wrote: > On 2014/12/24 18:21:15, Rouslan S. (back Jan 2nd) wrote: ...
5 years, 12 months ago (2014-12-25 21:04:58 UTC) #8
yukawa
On 2014/12/25 21:04:58, Rouslan S. (back Jan 2nd) wrote: > On 2014/12/25 09:41:07, Yuki wrote: ...
5 years, 12 months ago (2014-12-26 02:45:29 UTC) #9
Shu Chen
This cl seems only addressing the issue on Chrome OS. The cl description mentioned Android ...
5 years, 11 months ago (2014-12-31 02:02:29 UTC) #10
please use gerrit instead
I haven't addressed previous suggestions yet, but here is a few clarifying remarks. On 2014/12/31 ...
5 years, 11 months ago (2015-01-02 20:41:20 UTC) #11
please use gerrit instead
https://codereview.chromium.org/826713002/diff/40001/ui/keyboard/keyboard_controller.cc File ui/keyboard/keyboard_controller.cc (right): https://codereview.chromium.org/826713002/diff/40001/ui/keyboard/keyboard_controller.cc#newcode482 ui/keyboard/keyboard_controller.cc:482: proxy_->GetInputMethod()->GetTextInputClient()->OnKeyboardBoundsUnchanged(); On 2014/12/31 02:02:28, Shu Chen wrote: > should ...
5 years, 11 months ago (2015-01-02 20:41:28 UTC) #12
Shu Chen
On 2015/01/02 20:41:20, Rouslan Solomakhin wrote: > I haven't addressed previous suggestions yet, but here ...
5 years, 11 months ago (2015-01-04 03:45:50 UTC) #13
yukawa
On 2015/01/04 03:45:50, Shu Chen wrote: > Unlike Windows 7, Windows 8 has full virtual ...
5 years, 11 months ago (2015-01-04 14:55:08 UTC) #14
please use gerrit instead
On 2015/01/04 14:55:08, yukawa wrote: > Having said that, I think we can focus on ...
5 years, 11 months ago (2015-01-04 20:44:58 UTC) #15
please use gerrit instead
Yohei, Shu, PTAL Patch Set 2. I've moved OnKeyboardBoundsUnchanged from TextInputClient to InputMethodObserver. I'm still ...
5 years, 11 months ago (2015-01-05 19:36:46 UTC) #18
Shu Chen
https://codereview.chromium.org/826713002/diff/100001/ui/base/ime/input_method.h File ui/base/ime/input_method.h (right): https://codereview.chromium.org/826713002/diff/100001/ui/base/ime/input_method.h#newcode166 ui/base/ime/input_method.h:166: virtual void SetSupportsOnScreenKeyboard(bool supported) = 0; As commented by ...
5 years, 11 months ago (2015-01-06 02:19:51 UTC) #19
Shu Chen
https://codereview.chromium.org/826713002/diff/100001/ui/base/ime/input_method.h File ui/base/ime/input_method.h (right): https://codereview.chromium.org/826713002/diff/100001/ui/base/ime/input_method.h#newcode166 ui/base/ime/input_method.h:166: virtual void SetSupportsOnScreenKeyboard(bool supported) = 0; A solution just ...
5 years, 11 months ago (2015-01-06 02:35:25 UTC) #20
Shu Chen
https://codereview.chromium.org/826713002/diff/100001/ui/base/ime/input_method.h File ui/base/ime/input_method.h (right): https://codereview.chromium.org/826713002/diff/100001/ui/base/ime/input_method.h#newcode166 ui/base/ime/input_method.h:166: virtual void SetSupportsOnScreenKeyboard(bool supported) = 0; On 2015/01/06 02:35:25, ...
5 years, 11 months ago (2015-01-06 02:38:39 UTC) #21
Shu Chen
+stevet@/bshe@, who is owners for VK of ChromeOS. Steve/Biao, do you have any thoughts? Thanks, ...
5 years, 11 months ago (2015-01-07 03:46:29 UTC) #22
bshe
Using InputMethodDelegate seems will create a circular dependency between ui/wm and ui/keyboard too. Maybe we ...
5 years, 11 months ago (2015-01-07 17:16:08 UTC) #24
please use gerrit instead
5 years, 8 months ago (2015-04-23 16:38:18 UTC) #25
Going to close for now, as the priority is low. Will dig this up again when I
resume work on it.

Powered by Google App Engine
This is Rietveld 408576698