Should not show ellipsis when the password is in masked state |
||||||||
Issue descriptionIf the password is long enough, we currently show an ellipsis for that even in the masked state. According to maxwalker@, the ideal behavior should be as follows: 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 6
,
Dec 6
Re comment#1: I tried that and the tricky issue is the long password would always show one more character than the short one. I think that's because the space which was reserved for the ellipsis is then used by the one extra character.
,
Dec 7
I can't reproduce that behavior. What I observe - 17 chars password: either 15 bullets + ellipsis, or 16 bullets without "text-overflow: ellipsis;" - 16 chars password: 16 bullets with or without "text-overflow: ellipsis;" Thus, I just get max 16 bullets. Seems to be desired.
,
Dec 7
Okay. I just realized that the default length of generated password is 15. So all the "short passwords" I referred to have 15 characters. That's why the long one shows one more without "text-overflow: ellipsis;".
,
Dec 10
Thus, nothing speaks against the proposed fix, right?
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/503b1b9f7723226fbfe248f9cb876af17bb1c2d2 commit 503b1b9f7723226fbfe248f9cb876af17bb1c2d2 Author: Yue Cen <rsgingerrs@chromium.org> Date: Tue Dec 11 03:38:37 2018 Password manager: Remove ellipsis in masked state Bug: 912321 Change-Id: Ibfddb6628b06fe5111f873cf7716a210d47deb36 Reviewed-on: https://chromium-review.googlesource.com/c/1370369 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Cr-Commit-Position: refs/heads/master@{#615414} [modify] https://crrev.com/503b1b9f7723226fbfe248f9cb876af17bb1c2d2/chrome/browser/resources/settings/autofill_page/password_list_item.html
,
Dec 11
Could you try to merge it as well?
,
Dec 11
Sure. Request a merge to M72.
,
Dec 12
Able to reproduce the issue on Ubuntu 17.10, Mac 10.14.0 and windows 10 using chrome build without fix 73.0.3632.0. Verified the fix on Ubuntu 17.10, Mac 10.14.0 and windows 10 using Chrome version #73.0.3638.0. Attaching screen shot for reference. Observed that a Password shows a bullet for each character until the max number of bullets is reached. Hence, the issue is working as expected. Adding the verified labels. Thanks...!
,
Dec 12
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f553dbf21c6d542ff43787923bfa1361e3632bcd commit f553dbf21c6d542ff43787923bfa1361e3632bcd Author: Yue Cen <rsgingerrs@chromium.org> Date: Wed Dec 12 20:45:20 2018 [Merge to M72] Password manager: Remove ellipsis in masked state TBR=rsgingerrs@chromium.org (cherry picked from commit 503b1b9f7723226fbfe248f9cb876af17bb1c2d2) Bug: 912321 Change-Id: Ibfddb6628b06fe5111f873cf7716a210d47deb36 Reviewed-on: https://chromium-review.googlesource.com/c/1370369 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#615414} Reviewed-on: https://chromium-review.googlesource.com/c/1374353 Reviewed-by: Yue Cen <rsgingerrs@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#304} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/f553dbf21c6d542ff43787923bfa1361e3632bcd/chrome/browser/resources/settings/autofill_page/password_list_item.html
,
Dec 13
Verified the fix on Ubuntu 17.10, Mac 10.14.0 and windows 10 using chrome version #72.0.3626.17. Attaching screenshot for reference. Observed that a Password shows a bullet for each character until the max number of bullets is reached. Hence, the issue is working as expected. Adding the verified labels. Thanks...!
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f553dbf21c6d542ff43787923bfa1361e3632bcd Commit: f553dbf21c6d542ff43787923bfa1361e3632bcd Author: rsgingerrs@chromium.org Commiter: rsgingerrs@chromium.org Date: 2018-12-12 20:45:20 +0000 UTC [Merge to M72] Password manager: Remove ellipsis in masked state TBR=rsgingerrs@chromium.org (cherry picked from commit 503b1b9f7723226fbfe248f9cb876af17bb1c2d2) Bug: 912321 Change-Id: Ibfddb6628b06fe5111f873cf7716a210d47deb36 Reviewed-on: https://chromium-review.googlesource.com/c/1370369 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Yue Cen <rsgingerrs@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#615414} Reviewed-on: https://chromium-review.googlesource.com/c/1374353 Reviewed-by: Yue Cen <rsgingerrs@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#304} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by maxwalker@chromium.org
, Dec 6