Regression : Blink of scrollbar is seen on reloading the chrome://settings/passwords page |
|||||||
Issue descriptionVersion: 54.0.2791.0 OS: Ubuntu 14.04,Windows What steps will reproduce the problem? (1)Launch chrome -> Go to "chrome://settings/passwords" page (2)Now reload the page and observe Blink of scrollbar is seen on passwords overlay (Please refer Video and screenshot) Expected: No Blink of scrollbar should be seen on on passwords overlay Actual: Instead Blink of scrollbar is seen his is Regression Issue broken in M-53 Manual good and Bad Builds: Good Build: 52.0.2711.0 Bad Build: 52.0.2712.0 Below is the Bisect info : CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/79105d42633f347ff3950ac985bc4397bd8c6e0a..53f17391c1e7884791dc02b14cbba2e6dd65ff91 Suspecting https://codereview.chromium.org/1193143003 from above Changelog @xunlu : Please feel free to re-assign if its not related to your change
,
Jul 12 2016
Vaclav, could you take a look?
,
Jul 13 2016
,
Jul 18 2016
,
Jul 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33bd96dd8b9c462fe5a83d43a6ce535321907ca5 commit 33bd96dd8b9c462fe5a83d43a6ce535321907ca5 Author: vabr <vabr@chromium.org> Date: Wed Jul 20 18:04:30 2016 Hide the DIV containing password import/export buttons The import/export feature for passwords is currently behind a flag. However, the settings WebUI hides the controls instead of the containing DIV. As a result, on loading the settings, the space for the controls is briefly taken into consideration and a vertical scrollbar shows up. This CL hides the containing DIV instead of the individual controls. Test steps: With chrome://flags/#password-import-export enabled, observe that about:settings/passwords looks the same as without the patch. With chrome://flags/#password-import-export disabled, observe that during reloading of about:settings/passwords, no vertical scroll bar is flashing/seen. BUG= 626634 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2154223002 Cr-Commit-Position: refs/heads/master@{#406616} [modify] https://crrev.com/33bd96dd8b9c462fe5a83d43a6ce535321907ca5/chrome/browser/resources/options/password_manager.html [modify] https://crrev.com/33bd96dd8b9c462fe5a83d43a6ce535321907ca5/chrome/browser/resources/options/password_manager.js
,
Jul 21 2016
The issue is now fixed on ToT. I'll try merge requests after a day or two.
,
Jul 22 2016
,
Jul 22 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/110f6b0a9c2fd64a1576842756e3bd0f6e8b2820 commit 110f6b0a9c2fd64a1576842756e3bd0f6e8b2820 Author: Vaclav Brozek <vabr@chromium.org> Date: Fri Jul 22 08:41:41 2016 [Merge to 2785] Hide the DIV containing password import/export buttons The import/export feature for passwords is currently behind a flag. However, the settings WebUI hides the controls instead of the containing DIV. As a result, on loading the settings, the space for the controls is briefly taken into consideration and a vertical scrollbar shows up. This CL hides the containing DIV instead of the individual controls. Test steps: With chrome://flags/#password-import-export enabled, observe that about:settings/passwords looks the same as without the patch. With chrome://flags/#password-import-export disabled, observe that during reloading of about:settings/passwords, no vertical scroll bar is flashing/seen. BUG= 626634 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2154223002 Cr-Commit-Position: refs/heads/master@{#406616} (cherry picked from commit 33bd96dd8b9c462fe5a83d43a6ce535321907ca5) Review URL: https://codereview.chromium.org/2176573002 . Cr-Commit-Position: refs/branch-heads/2785@{#291} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/110f6b0a9c2fd64a1576842756e3bd0f6e8b2820/chrome/browser/resources/options/password_manager.html [modify] https://crrev.com/110f6b0a9c2fd64a1576842756e3bd0f6e8b2820/chrome/browser/resources/options/password_manager.js
,
Jul 22 2016
Merged, will watch the beta builders.
,
Jul 22 2016
Beta builders look good, only Linux is a bit behind, so not able to tell yet.
,
Jul 22 2016
Requesting merge of r406616 to M-52 (branch 2743). This might be pushing it, but I'll try, because the issue is in 52 already. If it's not appropriate for 52 any more, that's fair enough.
,
Jul 22 2016
M52 is already in Stable and bar is VERY high. We're going to have M52 refresh Stable release next week and we can take this change in ONLY if it is critical, baked/verified in Canary and safe to merge. Please confirm this. Thank you.
,
Jul 22 2016
FYI: We're planning to cut M52 Stable RC next week on Monday @ 5:00 PM PDT.
,
Jul 22 2016
Hi, thanks for the message. This is not critical, it is just a minor UI glitch. I am withdrawing my stable merge request based on #13.
,
Jul 26 2016
Verified this issue on Windows-7, Ubuntu 14.04 and Mac OS 10.11.5 using chrome latest Dev M53-53.0.2785.30 by following steps mentioned in the original comment. Observed no blink of scrollbar is seen on clicking "Manage Passwords" under chrome://settings. Hence adding TE-Verified label. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by ajha@chromium.org
, Jul 8 2016