New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 771878 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 753806



Sign in to add a comment

Regression: Username and Password text looks weird on save password bubble (Win/Linux/CrOS)

Reported by dchau...@etouch.net, Oct 5 2017

Issue description

Chrome Version: 63.0.3233.0 (Official Build)e8cc7650d44155e942550b9c730eafc9c5ba8ab6-refs/heads/master@{#506599} 32/64-bit.
OS: Windows(7,8,10)

What steps will reproduce the problem?
1. Launch chrome and sign in to www.gmail.com with valid credentials.
2. Now multiple times click on show password icon on save password bubble and observe.

Username and Password text looks weird.
Username and Password text should seen properly.

This is a regression issue, broken in M-63 series, will soon update other info.
 
Labels: hasbisect-per-revision OS-Linux
Owner: kolos@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: Regression: Username and Password text looks weird on save password bubble. (was: Regression: Username and Password looks weird on save password bubble.)
Below is manual regression range.

Good build: 63.0.3227.0 
Bad build: 63.0.3228.0

You are probably looking for a change made after 505351 (known good), but no later than 505352 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/3f20c4c038611f40f0915e27b18086d5fea9e18e..dc75dfa9a1c98141af036140090e698deb64c2c1

Suspect: https://chromium.googlesource.com/chromium/src/+/dc75dfa9a1c98141af036140090e698deb64c2c1

@kolos: Kindly help to reassign, if your changes are not related to this issue.

Note: This issue is not seen on Mac OS.

Kindly review the attached screen-cast for reference.
Actual behavior.mp4
1.8 MB View Download
Expected behavior.mp4
669 KB View Download
Labels: ReleaseBlock-Beta
Tagging with blocker label, please undo if not the case.

Comment 3 by kolos@chromium.org, Oct 5 2017

Labels: -ReleaseBlock-Beta
Status: WontFix (was: Assigned)
This is expected look of save prompt. No worries :) Thanks for pointing out. 
With respond to comment #3: 

After multiple times clicking on eye icon (show password icon) on save password bubble, texts 'Username' and 'Password' are getting bold and looks weird in font.

Kindly refer the attached Actual behavior screen-cast and Password_bubble_Actual screenshot for the reference.


Password_bubble_Actual.png
6.7 KB View Download

Comment 5 by kolos@chromium.org, Oct 6 2017

Blocking: 753806
Cc: vasi...@chromium.org
Status: Available (was: WontFix)
oh, got it. Thanks. 

vasilii: something is wrong even with old layout. I tried to copy code from bookmarks https://cs.chromium.org/chromium/src/chrome/browser/ui/views/bookmarks/bookmark_bubble_view.cc?rcl=a77261a2b7367519e13a15880c5ab6174c1bbf1b&l=281  

The bookmarks set row_height based on fonts (https://cs.chromium.org/chromium/src/chrome/browser/ui/views/harmony/textfield_layout.cc?gsn=AddFirstTextfieldRow&l=22). 

I will take a look if I have time. 
I think it's a mistake. I don't see how font is connected with the height of the row. The font should be hardcoded and the height determined dynamically by the layout.

Comment 7 by kolos@chromium.org, Oct 6 2017

Components: Internals>Views>Desktop
Adding Internals>Views>Desktop in case relevant people know the problem.

Comment 8 by kolos@chromium.org, Oct 6 2017

For Views folks: Toggling passwords visibility (https://cs.chromium.org/chromium/src/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc?rcl=08e161eecbdb3293fdbfbb4ce5dc736391024aad&l=539) in the bubble should not affect on font, but for some reason it does.

Comment 9 by kolos@chromium.org, Oct 10 2017

Cc: sadrul@chromium.org msw@chromium.org
Owner: msw@chromium.org
Status: Assigned (was: Available)
adding views folks to the thread. The label element (https://cs.chromium.org/chromium/src/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc?rcl=26c452fd56661c7c0597385c6882488d4b117ffa&l=270) should stay unchanged when a user clicks at another element (the eye icon https://cs.chromium.org/chromium/src/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc?rcl=08e161eecbdb3293fdbfbb4ce5dc736391024aad&l=539).

Comment 10 by kolos@chromium.org, Oct 10 2017

Cc: kolos@chromium.org

Comment 11 by msw@chromium.org, Oct 10 2017

Owner: kolos@chromium.org
TogglePasswordVisibility() calls CreateAndSetLayout() which re-adds all the views?
It seems possible that the views are being stacked and painted on top of one another?
The view shouldn't need to layout, let alone recreate the layout on this interaction.
Please take another look.
Maxim, are you working on this? It looks pretty weird if you click the eye icon many times. This definitely should be merged.

Comment 13 by kolos@chromium.org, Oct 17 2017

Status: Started (was: Assigned)
Started. 

My suggestion is to implement mutable ComboboxModel. Let's discuss in person.

Comment 14 by kolos@chromium.org, Oct 17 2017

Summary: Regression: Username and Password text looks weird on save password bubble (Win/Linux/CrOS) (was: Regression: Username and Password text looks weird on save password bubble.)
Labels: OS-Chrome
Maxim, we have labels which are as good as the title :)

Comment 16 by kolos@chromium.org, Oct 17 2017

Labels are not shown when you see the list of bugs that block the given bug.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 18 2017

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

commit 356eb23202702b62eeb36c3727a5db816704c181
Author: Maxim Kolosovskiy <kolos@chromium.org>
Date: Wed Oct 18 14:00:04 2017

[Password Manager] Don't recreate layout of a save prompt when passwords visiblity has changed

Bug:  771878 
Change-Id: Idf39c939154da6d143934858ba88c96d5312fa4a
Reviewed-on: https://chromium-review.googlesource.com/723319
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509761}
[modify] https://crrev.com/356eb23202702b62eeb36c3727a5db816704c181/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc

Comment 18 by kolos@chromium.org, Oct 19 2017

Labels: Merge-Request-63
Without this fix, UI of save password prompt looks weird. Should be fixed. 
Project Member

Comment 19 by sheriffbot@chromium.org, Oct 20 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

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

Comment 20 by bugdroid1@chromium.org, Oct 20 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2dcedcf0bb2126cff1b6a6eb2111db5c5aa5b306

commit 2dcedcf0bb2126cff1b6a6eb2111db5c5aa5b306
Author: Maxim Kolosovskiy <kolos@chromium.org>
Date: Fri Oct 20 09:27:32 2017

[Password Manager] Don't recreate layout of a save prompt when passwords visiblity has changed

Bug:  771878 
Change-Id: Idf39c939154da6d143934858ba88c96d5312fa4a
Reviewed-on: https://chromium-review.googlesource.com/723319
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509761}(cherry picked from commit 356eb23202702b62eeb36c3727a5db816704c181)
Reviewed-on: https://chromium-review.googlesource.com/729864
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#106}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/2dcedcf0bb2126cff1b6a6eb2111db5c5aa5b306/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc

Comment 21 by kolos@chromium.org, Oct 20 2017

Status: Fixed (was: Started)
Labels: TE-Verified-M64 TE-Verified-64.0.3247.0
Update:
Retested this issue on Windows-(7,8,10) machine using latest Canary build # 64.0.3247.0 (Official Build) and fix is working as expected i.e. Username and Password text seen properly after multiple times clicking on eye (show password) icon.

Attaching screen-cast for the same.
Latest Canary behavior.mp4
1.8 MB View Download
Labels: TE-Verified-M63 TE-Verified-63.0.3239.16
Update:
Retested this issue on Windows-(7,8,10) machine using Dev build # 63.0.3239.16 (Official Build) and fix is working as expected i.e. Username and Password text seen properly after multiple times clicking on eye (show password) icon.

Attaching screen-cast for the same.
Dev_behavior.mp4
1.5 MB View Download

Sign in to add a comment