New issue
Advanced search Search tips

Issue 853748 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 25
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 853794



Sign in to add a comment

Password accessory sheet entries show favicons of associated site

Project Member Reported by fhorschig@chromium.org, Jun 18 2018

Issue description

Every entry in the password accessory sheet needs to display an icon next to it (as visual separator and to show that it's associated with the current site).

If no icon is available, fall back to globe_favicon.png

(For padding, see:
https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZRT3KyGvNYc6/files/MCEQIFEoArGIwEvAawK07K_1wMSJbxAVNpw)
 
Blocking: 853794
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 25

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

commit ca8690dca6bc15a090a754dd638c414bb2bf0025
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Wed Jul 25 14:16:46 2018

[Android] Display favicons in password accessory sheet

This CL adds an image view to all usernames in the passwords accessory
sheet which separates username/password groups visually.
It also indicates the origin of the credentials.

The icons are loaded by origin on the native side. If icons are
requested multiple times, they will be grouped by origin and result
in only one request to the FaviconService.
Already requested icons are cached - when one of these is requested,
the callback will return synchronously instead of async.

Bug:  853748 
Change-Id: Ia97c5c62710daf709d7ee19d732d2e226ac7634f
Reviewed-on: https://chromium-review.googlesource.com/1138319
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577876}
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/java/res/layout/password_accessory_sheet_suggestion.xml
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetCoordinator.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewBinder.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryIntegrationTest.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewTest.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/ca8690dca6bc15a090a754dd638c414bb2bf0025/chrome/test/BUILD.gn

Labels: Merge-Request-69
Status: Fixed (was: Assigned)
Reason for merge request: this CL fixes the layout in order to add favicons. In particular, it ensures that RecyclerView items are set to the correct default states.
Although quite large, the majority of this CL is very tightly scoped.
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 26

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 26

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fc132ef31eb921f772852b880c88e4b1516096c

commit 3fc132ef31eb921f772852b880c88e4b1516096c
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Jul 26 17:00:03 2018

[Android] Display favicons in password accessory sheet

This CL adds an image view to all usernames in the passwords accessory
sheet which separates username/password groups visually.
It also indicates the origin of the credentials.

The icons are loaded by origin on the native side. If icons are
requested multiple times, they will be grouped by origin and result
in only one request to the FaviconService.
Already requested icons are cached - when one of these is requested,
the callback will return synchronously instead of async.

TBR=fhorschig@chromium.org

(cherry picked from commit ca8690dca6bc15a090a754dd638c414bb2bf0025)

Bug:  853748 
Change-Id: Ia97c5c62710daf709d7ee19d732d2e226ac7634f
Reviewed-on: https://chromium-review.googlesource.com/1138319
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577876}
Reviewed-on: https://chromium-review.googlesource.com/1151631
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#114}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/java/res/layout/password_accessory_sheet_suggestion.xml
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/KeyboardAccessoryData.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetCoordinator.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewBinder.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryIntegrationTest.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessorySheetViewTest.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/android/junit/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingControllerTest.java
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/browser/password_manager/password_accessory_controller.h
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/3fc132ef31eb921f772852b880c88e4b1516096c/chrome/test/BUILD.gn

Status: Verified (was: Fixed)
Verified with amazon.com in 70.0.3510.0. Amazon's favicon is shown in the keyboard accessory sheet.

Sign in to add a comment