New issue
Advanced search Search tips

Issue 689574 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

separator in website settings popup no longer extends all the way to the edge

Project Member Reported by est...@chromium.org, Feb 7 2017

Issue description

in 56, it goes all the way to the edge.
in 58, it doesn't.

I suspect crrev.com/57b697aa2605b6bbcb97352a32ccdb28204739e9
 
separator56.png
13.9 KB View Download
DY74Xd3fmdr.png
15.4 KB View Download
Labels: -M-58 M-57
drat I totally missed that - thanks for reporting! The harmony dialog will need a fix for this too.

(turns out: tweaking dialog layouts behind a runtime flag is kinda tricky :/)
Status: Started (was: Assigned)
before, after, and a pixel diff (Harmony)

https://codereview.chromium.org/2686983002
before.png
218 KB View Download
after.png
217 KB View Download
diff.png
210 KB View Download
Here is Win/non-harmony before/after. Note the dialog gets slightly narrower, which is a closer match to the m56 version as well.
before_win.png
373 KB View Download
after_win.png
369 KB View Download
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 9 2017

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

commit 3b76fcf9516aed2d90a61ae035cfdf2b2a18e08c
Author: tapted <tapted@chromium.org>
Date: Thu Feb 09 06:17:10 2017

Ensure the line on the origin info bubble extends the full width of the bubble.

This regressed in r444474 which was necessary to adapt the bubble
margins to support Harmony layout (i.e. since margins are no longer a
compile-time constant).

The separator line, however, needs to ignore the margins completely. So
move the bubble margin into the classes doing layout above and below the
separator.

BUG= 689574 

Review-Url: https://codereview.chromium.org/2686983002
Cr-Commit-Position: refs/heads/master@{#449222}

[modify] https://crrev.com/3b76fcf9516aed2d90a61ae035cfdf2b2a18e08c/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
[modify] https://crrev.com/3b76fcf9516aed2d90a61ae035cfdf2b2a18e08c/chrome/browser/ui/views/website_settings/website_settings_popup_view.h

Cc: ellyjo...@chromium.org
 Issue 657264  has been merged into this issue.

Comment 6 by tapted@chromium.org, Feb 12 2017

Labels: Merge-Request-57
Verified fixed in Version 58.0.3010.0 canary, requesting merge of r449222 to m-57
Project Member

Comment 7 by sheriffbot@chromium.org, Feb 12 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

Comment 8 by bugdroid1@chromium.org, Feb 12 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39df7ec4a3cf18ffd93ac26ab1472d5031f4b9d5

commit 39df7ec4a3cf18ffd93ac26ab1472d5031f4b9d5
Author: Trent Apted <tapted@chromium.org>
Date: Sun Feb 12 22:52:20 2017

[merge-m57] Ensure the line on the origin info bubble extends the full width of the bubble.

This regressed in r444474 which was necessary to adapt the bubble
margins to support Harmony layout (i.e. since margins are no longer a
compile-time constant).

The separator line, however, needs to ignore the margins completely. So
move the bubble margin into the classes doing layout above and below the
separator.

BUG= 689574 

Review-Url: https://codereview.chromium.org/2686983002
Cr-Commit-Position: refs/heads/master@{#449222}
(cherry picked from commit 3b76fcf9516aed2d90a61ae035cfdf2b2a18e08c)

Review-Url: https://codereview.chromium.org/2692773002 .
Cr-Commit-Position: refs/branch-heads/2987@{#467}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/39df7ec4a3cf18ffd93ac26ab1472d5031f4b9d5/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
[modify] https://crrev.com/39df7ec4a3cf18ffd93ac26ab1472d5031f4b9d5/chrome/browser/ui/views/website_settings/website_settings_popup_view.h

Comment 9 by tapted@chromium.org, Feb 12 2017

Status: Fixed (was: Started)
Labels: TE-Verified-57.0.2987.54 TE-Verified-M57
Verified this issue on Ubuntu 14.04 and Windows-10 using chrome latest M57-57.0.2987.54. By clicking on secure lock icon from the omnibox observed the dialog gets narrower as expected. Hence adding TE-Verified label.
689574.png
87.4 KB View Download
Components: UI>Browser>Bubbles>PageInfo

Sign in to add a comment