New issue
Advanced search Search tips

Issue 865479 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Footer height is 36dp on the new dropdown

Project Member Reported by ftirelo@chromium.org, Jul 19

Issue description

Chrome Version: M69
OS: desktop

What steps will reproduce the problem?
(1) Enable chrome://flags/#upcoming-ui-features
(2) Navigate to https://rsolomakhin.github.io/autofill, scroll up to the top of the page and click on field "Name", section "Profile Autofill" (if you don't see the autofill dropdown, go to chrome://settings/autofill and add a new address)

What is the expected result?
Footer height (row with "Manage addresses...") should be 40px.

What happens instead?
It's 36px.

This was broken by http://crrev.com/fa89015c94aa96aed4bcf1452b8c808de3ab627c
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20

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

commit 368ce422837110d70ee2b14b42fac364199cb0c0
Author: Tommy Martino <tmartino@chromium.org>
Date: Fri Jul 20 02:57:24 2018

[Autofill Views] Restoring footer to 40px

This CL addresses a UX regression caused in this revision:
https://chromium.googlesource.com/chromium/src/+/fa89015c94aa96aed4bcf1452b8c808de3ab627c

Specifically, the extra height which compensates for corner rounding in
the footer was removed. In pixel terms, this shrunk the footer row from
40px to 36px. This CL reintroduces the extra height.

Change-Id: Ib5d6c1d0a8af1bd63c960597654f85982e280e3a
Bug:  865479 
Reviewed-on: https://chromium-review.googlesource.com/1143688
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576762}
[modify] https://crrev.com/368ce422837110d70ee2b14b42fac364199cb0c0/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc

Labels: -Pri-3 Merge-Request-69 OS-Chrome OS-Linux OS-Mac OS-Windows Pri-2
Requesting merge for M69. This addresses a user-facing regression introduced shortly before branch point. The fix is very low risk: it's one line, and it's restoring behavior that existed stably for several weeks prior to the regression.
Status: Fixed (was: Assigned)
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 21

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
Please merge your change to M69 branch 3497 by 4:00 PM PT today, so we can pick it up for this week M69 Dev release. Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 23

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

commit 1b5dd7ce82f97dfd81f07649fff0c7dc76c8e6d6
Author: Tommy Martino <tmartino@chromium.org>
Date: Mon Jul 23 18:58:17 2018

[Autofill Views] Restoring footer to 40px

This CL addresses a UX regression caused in this revision:
https://chromium.googlesource.com/chromium/src/+/fa89015c94aa96aed4bcf1452b8c808de3ab627c

Specifically, the extra height which compensates for corner rounding in
the footer was removed. In pixel terms, this shrunk the footer row from
40px to 36px. This CL reintroduces the extra height.

Change-Id: Ib5d6c1d0a8af1bd63c960597654f85982e280e3a
Bug:  865479 
Reviewed-on: https://chromium-review.googlesource.com/1143688
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#576762}(cherry picked from commit 368ce422837110d70ee2b14b42fac364199cb0c0)
Reviewed-on: https://chromium-review.googlesource.com/1147001
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#22}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/1b5dd7ce82f97dfd81f07649fff0c7dc76c8e6d6/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc

Labels: Needs-Feedback
Tried testing the issue on build without fix #69.0.3439.3 and latest chrome 70.0.3502.0 on Mac 10.12.6 by following below steps.

1.launched chrome.
2.Enable chrome://flags/#upcoming-ui-features
3.Navigate to https://rsolomakhin.github.io/autofill and clicked on field "Name", section "Profile Autofill" but could see same behaviour on both builds.
Attached screenshots for reference

ftirelo@ could please review the screenshot and confirm the issue, so that it would be really helpful for further verfication of the fix.

Thanks.!
70.0.3502.0.png
325 KB View Download
69.0.3493.0(2).png
311 KB View Download
Hi there,

The broken behavior was introduced by https://chromiumdash.appspot.com/commit/fa89015c94aa96aed4bcf1452b8c808de3ab627c and first shipped in the 69.0.3495.2 Canary. The goal of this bug was to restore the behavior seen before that patch.

Thus, we would expect the 69.0.3493.3 and 70.0.3502.0 screenshots to be the same.

Sign in to add a comment