Show website favicon in password manager |
|||||||||||
Issue description
,
Sep 28
Can you make a screeshot of the whole page (= whole content area)? It's difficult to assess only one column.
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4cb312703d70ef1fd7f4379d971ff848b5977b8a commit 4cb312703d70ef1fd7f4379d971ff848b5977b8a Author: Yue Cen <rsgingerrs@chromium.org> Date: Tue Oct 23 21:18:59 2018 [Settings] Add a component site-favicon to display site favicon This is a refactor CL and it should not change any existing UI. The affected pages are as follows: - On startup site entry in chrome://settings - chrome://settings/siteData - chrome://settings/searchEngines - chrome://settings/content/zoomLevels - chrome://settings/handlers - chrome://settings/content/usbDevices - chrome://settings/content/all - All the pages in "Content settings" that use site-list-entry, such as chrome://settings/content/paymentHandler Bug: 890118 Change-Id: I35e00406999cf30012db557b10dd418cef67a69d Reviewed-on: https://chromium-review.googlesource.com/c/1277995 Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#602099} [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/on_startup_page/startup_url_entry.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/on_startup_page/startup_url_entry.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/search_engines_page/search_engine_entry.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/search_engines_page/search_engine_entry.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/search_engines_page/search_engine_entry_css.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/settings_resources.grd [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/settings_shared_css.html [add] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_favicon.html [add] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_favicon.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/protocol_handlers.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_data.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_data_entry.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_data_entry.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_entry.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_entry.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_list_entry.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_settings_behavior.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/site_settings_behavior.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/usb_devices.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/browser/resources/settings/site_settings/zoom_levels.html [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/test/data/webui/settings/cr_settings_browsertest.js [add] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/chrome/test/data/webui/settings/site_favicon_test.js [modify] https://crrev.com/4cb312703d70ef1fd7f4379d971ff848b5977b8a/testing/buildbot/filters/webui_polymer2_browser_tests.filter
,
Oct 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7157ed9182e341ec40630d47154f11966232f6b commit f7157ed9182e341ec40630d47154f11966232f6b Author: Yue Cen <rsgingerrs@chromium.org> Date: Sat Oct 27 01:36:18 2018 [Password Manager] Add favicon in Password Manager in the settings Bug: 890118 Change-Id: I5bbb3093dfaabc0aa8d7e89fd2bf145f26884f42 Reviewed-on: https://chromium-review.googlesource.com/c/1239437 Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#603287} [modify] https://crrev.com/f7157ed9182e341ec40630d47154f11966232f6b/chrome/browser/resources/settings/passwords_and_forms_page/password_list_item.html [modify] https://crrev.com/f7157ed9182e341ec40630d47154f11966232f6b/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html [modify] https://crrev.com/f7157ed9182e341ec40630d47154f11966232f6b/chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html
,
Nov 14
Per UX's request, we are making the following changes: - Increase the padding between the icon and the domain like in chrome://history. - Make the URLS for the domain black (but still clickable). Please see the expected UI as follows.
,
Nov 15
Max, is this exactly what you wanted?
,
Nov 15
Repeating my comment from https://chromium-review.googlesource.com/c/chromium/src/+/1332258/1#message-bf5cf488e0f27b88cb92bcb5d40ad885eb39a973. Changing the font color makes this UI inconsistent with other places in Settings (see screenshot). We put a lot of effort on keeping those consistent see issue 849857 during the recent MD Refresh. Is there a good reason to only change the color here? Can we revert this to keep consistency with the original MD refresh specs? @namratakannan: Any thoughts?
,
Dec 3
Hi! Passwords should show a bullet for each character until the max number of bullets (which I think is 16) is reached: - Password with length x < max: show x characters - Password with length x = max: show max characters - Password with length x > max: show max characters Examples: - •••• (x = 4) - •••••• (x = 6) - •••••••••••••• (x = max) - •••••••••••••• (x > max)
,
Dec 5
Thanks for the reply. There is a tricky issue that if we remove the ellipsis, the long passwords show one more character than the short one. Since showing a ellipsis on a long password in masked state is the original behavior and it doesn't block the favicon feature, I would suggest we land the UI changes and create a separate bug for the ellipsis issue.
,
Dec 5
I guess what Max means is to edit getPassword_() function and instead repeat(this.item.entry.numCharactersInPassword) have something like repeat(min(this.item.entry.numCharactersInPassword, 16)). That magic const doesn't exist today. Therefore I agree that it's a separate feature, trivial though.
,
Dec 5
Thanks! Creating a separate bug SGTM2.
,
Dec 5
Bug created: crbug.com/912321
,
Dec 6
For some reason the last commit does not show up here. We would like to request a merge to M72 for this commit: https://chromium.googlesource.com/chromium/src/+/8b974f8e7a514acf61ed018d55565731ff5d02e5
,
Dec 6
This bug requires manual review: There is .grd file changes and we are only 53 days from stable. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b974f8e7a514acf61ed018d55565731ff5d02e5 commit 8b974f8e7a514acf61ed018d55565731ff5d02e5 Author: Yue Cen <rsgingerrs@chromium.org> Date: Thu Dec 06 21:20:15 2018 Password manager: Style changes in the website column - Increase the padding between the icon and the domain like in chrome://history. - Make the URLS for the domain black (but still clickable). Bug: 890118 Change-Id: If63006dced601b2891e9b0864b788808acc1faa8 Reviewed-on: https://chromium-review.googlesource.com/c/1332258 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Cr-Commit-Position: refs/heads/master@{#614491} [modify] https://crrev.com/8b974f8e7a514acf61ed018d55565731ff5d02e5/chrome/browser/resources/settings/autofill_page/passwords_shared_css.html
,
Dec 7
Oops, now it shows up. Request to merge comment#19 in M72.
,
Dec 7
Approving the CL in Comment 19 for 72. Branch 3626
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/632693796875df389ea693c8dbfeeab44b16cc2d commit 632693796875df389ea693c8dbfeeab44b16cc2d Author: Yue Cen <rsgingerrs@chromium.org> Date: Fri Dec 07 19:48:19 2018 [Merge to M72] Password manager: Style changes in the website column - Increase the padding between the icon and the domain like in chrome://history. - Make the URLS for the domain black (but still clickable). TBR=rsgingerrs@chromium.org (cherry picked from commit 8b974f8e7a514acf61ed018d55565731ff5d02e5) Bug: 890118 Change-Id: If63006dced601b2891e9b0864b788808acc1faa8 Reviewed-on: https://chromium-review.googlesource.com/c/1332258 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614491} Reviewed-on: https://chromium-review.googlesource.com/c/1368444 Reviewed-by: Yue Cen <rsgingerrs@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#146} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/632693796875df389ea693c8dbfeeab44b16cc2d/chrome/browser/resources/settings/autofill_page/passwords_shared_css.html
,
Dec 18
,
Dec 19
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision 632693796875df389ea693c8dbfeeab44b16cc2d was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/632693796875df389ea693c8dbfeeab44b16cc2d Commit: 632693796875df389ea693c8dbfeeab44b16cc2d Author: rsgingerrs@chromium.org Commiter: rsgingerrs@chromium.org Date: 2018-12-07 19:48:19 +0000 UTC [Merge to M72] Password manager: Style changes in the website column - Increase the padding between the icon and the domain like in chrome://history. - Make the URLS for the domain black (but still clickable). TBR=rsgingerrs@chromium.org (cherry picked from commit 8b974f8e7a514acf61ed018d55565731ff5d02e5) Bug: 890118 Change-Id: If63006dced601b2891e9b0864b788808acc1faa8 Reviewed-on: https://chromium-review.googlesource.com/c/1332258 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614491} Reviewed-on: https://chromium-review.googlesource.com/c/1368444 Reviewed-by: Yue Cen <rsgingerrs@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#146} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Dec 26
,
Dec 27
Just noticed this. So the merge violation was a false alarm, correct? |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by battre@chromium.org
, Sep 28