New issue
Advanced search Search tips

Issue 853742 link

Starred by 2 users
Status: Fixed
Owner:
fhorschig@chromium.org
Closed: Aug 1
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 853794



Sign in to add a comment

Keyboard accessory sheet needs to respect its scope

Project Member Reported by fhorschig@chromium.org, Jun 18 2018 
The accessory currently isn't cleaned up as soon as it should. It also persists when it's not relevant anymore (like in fullscreen mode or when the user stopped editing a field).

Therefore, ensure that
 - switching a tab hides it temporarily
 - entering the fullscreen mode hides it temporarily
 - not focusing any input fields hides it temporarily
 - it is destroyed/hidden when the native information provider is destroyed (i.e. PasswordAccessoryBridge)

Comment 1 by fhorschig@chromium.org, Jun 18 2018

Blocking: 853794

Comment 2 by fhorschig@chromium.org, Jun 18 2018 

Switching a tab is solved in https://crrev.com/c/1100760. (Wrong Bug ids used there).
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 29 2018 

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

commit 37a90315cca8eed768114ec8203989f756acdb02
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Fri Jun 29 15:15:32 2018

[Android, Crash] Hide accessory sheet during tab overview

This CL makes the ManualFillingMediator consider various cases of a tab
being hidden. Most importantly, it's now aware to when the tab overview
is opened which resolves a crash that happened in combination with the
HorizontalTabSwitcherAndroid feature.

Bug: 856523,  853742 ,  856033 
Change-Id: If09a3ac3551e2ebed1cfac6bb4870ff98d6c1ec1
Reviewed-on: https://chromium-review.googlesource.com/1118384
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571480}
[modify] https://crrev.com/37a90315cca8eed768114ec8203989f756acdb02/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryView.java
[modify] https://crrev.com/37a90315cca8eed768114ec8203989f756acdb02/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingMediator.java
[modify] https://crrev.com/37a90315cca8eed768114ec8203989f756acdb02/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingIntegrationTest.java
[modify] https://crrev.com/37a90315cca8eed768114ec8203989f756acdb02/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingTestHelper.java
[modify] https://crrev.com/37a90315cca8eed768114ec8203989f756acdb02/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryIntegrationTest.java

Comment 4 by fhorschig@chromium.org, Jul 5

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 

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

commit 4c9cbfb597cd0014ad5200b0a94cf0f13c466af6
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Wed Jul 18 08:13:03 2018

[Android] Change suggestions based on frame of focused field

With this CL, the renderer notifies the password accessory controller
when the focus moves to or away from valid input fields.

Bug:  854152 ,  854150 ,  854149 ,  853742 
Change-Id: I63f075ce238db8b77c784e945eea8ec83d8d4344
Reviewed-on: https://chromium-review.googlesource.com/1124466
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575977}
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_agent.mojom
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/core/common/BUILD.gn
[add] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/autofill/core/common/filling_status.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/components/password_manager/core/browser/password_manager_driver.h
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/4c9cbfb597cd0014ad5200b0a94cf0f13c466af6/content/public/test/test_renderer_host.h

Comment 6 by fhorschig@chromium.org, Jul 18 

Only missing case: hide on unfocus an input field. The rest seems to work as intended.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 18 

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

commit 82c224b0aa0eed806ded736f5ad3fed09257926e
Author: Ian Clelland <iclelland@chromium.org>
Date: Wed Jul 18 14:25:19 2018

Revert "[Android] Change suggestions based on frame of focused field"

This reverts commit 4c9cbfb597cd0014ad5200b0a94cf0f13c466af6.

Sorry for the revert -- this is failing on one of the ChromeOS builders:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests/7968

You can see several failed tests in this log output:
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8940668796232357776/+/steps/interactive_ui_tests/0/logs/All__x2f_PasswordManagerBrowserTestWithConditionalPopupViews.AutofillLoginSignupForm__x2f_1/0


Original change's description:
> [Android] Change suggestions based on frame of focused field
> 
> With this CL, the renderer notifies the password accessory controller
> when the focus moves to or away from valid input fields.
> 
> Bug:  854152 ,  854150 ,  854149 ,  853742 
> Change-Id: I63f075ce238db8b77c784e945eea8ec83d8d4344
> Reviewed-on: https://chromium-review.googlesource.com/1124466
> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575977}

TBR=vasilii@chromium.org,dvadym@chromium.org,piman@chromium.org,fhorschig@chromium.org,mkwst@chromium.org

Change-Id: I01cc79e8dc98e29cbdb1a61c6da5a4cf021fcbfb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  854152 ,  854150 ,  854149 ,  853742 
Reviewed-on: https://chromium-review.googlesource.com/1141825
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Commit-Queue: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576042}
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_agent.mojom
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/autofill/core/common/BUILD.gn
[delete] https://crrev.com/b644b96d5341d71f216f1ae6e551589903106aec/components/autofill/core/common/filling_status.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/components/password_manager/core/browser/password_manager_driver.h
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/82c224b0aa0eed806ded736f5ad3fed09257926e/content/public/test/test_renderer_host.h
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 18 

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

commit 6cab120126f8a1f7ddcaafb41d59e041a1c9e095
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Wed Jul 18 15:23:45 2018

Reland "[Android] Change suggestions based on frame of focused field"

This reverts commit 82c224b0aa0eed806ded736f5ad3fed09257926e.

Reason for revert: The linked build (7968) wasn't the first broken build. The first broken build was 7959 where https://crrev.com/c/1139057 reenabled this known-to-be-flaky test (see issue 849582). dullweber@ takes care of the new revert.

Original change's description:
> Revert "[Android] Change suggestions based on frame of focused field"
> 
> This reverts commit 4c9cbfb597cd0014ad5200b0a94cf0f13c466af6.
> 
> Sorry for the revert -- this is failing on one of the ChromeOS builders:
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20ChromiumOS%20MSan%20Tests/7968
> 
> You can see several failed tests in this log output:
> https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8940668796232357776/+/steps/interactive_ui_tests/0/logs/All__x2f_PasswordManagerBrowserTestWithConditionalPopupViews.AutofillLoginSignupForm__x2f_1/0
> 
> 
> Original change's description:
> > [Android] Change suggestions based on frame of focused field
> > 
> > With this CL, the renderer notifies the password accessory controller
> > when the focus moves to or away from valid input fields.
> > 
> > Bug:  854152 ,  854150 ,  854149 ,  853742 
> > Change-Id: I63f075ce238db8b77c784e945eea8ec83d8d4344
> > Reviewed-on: https://chromium-review.googlesource.com/1124466
> > Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
> > Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> > Reviewed-by: Antoine Labour <piman@chromium.org>
> > Reviewed-by: Mike West <mkwst@chromium.org>
> > Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575977}
> 
> TBR=vasilii@chromium.org,dvadym@chromium.org,piman@chromium.org,fhorschig@chromium.org,mkwst@chromium.org
> 
> Change-Id: I01cc79e8dc98e29cbdb1a61c6da5a4cf021fcbfb
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  854152 ,  854150 ,  854149 ,  853742 
> Reviewed-on: https://chromium-review.googlesource.com/1141825
> Reviewed-by: Ian Clelland <iclelland@chromium.org>
> Commit-Queue: Ian Clelland <iclelland@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576042}

TBR=vasilii@chromium.org,dvadym@chromium.org,iclelland@chromium.org,piman@chromium.org,fhorschig@chromium.org,mkwst@chromium.org

Change-Id: Idaadebd9b00179935c3f6347cc669c0811a912e4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  854152 ,  854150 ,  854149 ,  853742 
Reviewed-on: https://chromium-review.googlesource.com/1141885
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576066}
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/renderer/autofill/fake_mojo_password_manager_driver.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/renderer/autofill/fake_mojo_password_manager_driver.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_agent.mojom
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types.typemap
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/core/common/BUILD.gn
[add] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/autofill/core/common/filling_status.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/content/browser/content_password_manager_driver_unittest.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/components/password_manager/core/browser/password_manager_driver.h
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/content/public/test/test_renderer_host.cc
[modify] https://crrev.com/6cab120126f8a1f7ddcaafb41d59e041a1c9e095/content/public/test/test_renderer_host.h

Comment 9 by fhorschig@chromium.org, Aug 1

Status: Fixed (was: Started)
All major changes are completed. If you find that the keyboard isn't dismissed where it should be, please make it a new bug with clear repro steps.
►

Sign in to add a comment