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

Issue 761301 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug-Regression

Blocking:
issue 741083



Sign in to add a comment

Regression: Pictures are seen out-of-box in changePicture overlay when font size is 'Very large'

Project Member Reported by rkalavakuntla@chromium.org, Sep 1 2017

Issue description

Chrome Version:62.0.3201.0/9900.0.0 dev-channel Peppy,Reks,Candy
OS:Chrome OS

What steps will reproduce the problem?
(1)Sign into user ->Open chrome://settings and set the font size to 'Very large'
(2)Go to ChangePicture Overlay ->scroll and Observe the bottom pictures(kindly refer video)

Actual:Profile pictures are seen out of changePicture overlay
Expected:Profile pictures shouldn't be seen out-of-box

This is a Regression issue as same is working fine in M-61

Note:Issue is not applicable to Linux,Windows OS

@stevenjb: Please confirm the issue.
 
Actual.webm
1.4 MB View Download
Blocking: 741083
Cc: reve...@chromium.org
Labels: -Pri-1 -M-62 M-63 Pri-2
I don't think we need to fix this for 61/62. We have a bunch of large font layout issues in Settings that we will eventually get to.

Cc: steve...@chromium.org
Owner: reve...@chromium.org
I'll take this as I fixed a similar issue recently.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 5 2017

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

commit 6a69033c6082f3fc61ffbe7876eb88a83e44d08b
Author: David Reveman <reveman@chromium.org>
Date: Tue Sep 05 22:11:17 2017

Change Picture: Improve layout more.

Takes font-size based padding for header into account and also
sets top position for container to ensure that viewport alignment
is still correct even if assumptions about header height are not
perfect.

Bug:  761301 ,  755929 
Test: verified layout is good at all font sizes
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ifbcf8301571fb6313829ef39b1c74eb6926550e3
Reviewed-on: https://chromium-review.googlesource.com/648111
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499767}
[modify] https://crrev.com/6a69033c6082f3fc61ffbe7876eb88a83e44d08b/chrome/browser/resources/settings/people_page/change_picture.html

Labels: Merge-Request-62 Merge-Request-61 M-61 M-62
Status: Fixed (was: Assigned)
Project Member

Comment 5 by sheriffbot@chromium.org, Sep 6 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Request-62 Merge-Approved-62
Merge approved for 62, please ensure this CL makes it into ToT Chrome OS first, this does not appear to be in the last successful Chrome PFQ run, but it should be in by tomorrow (63.0.3207.0+ should pass PFQ first).
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 8 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1391f9e9f2528d100ccc1f14f620f3efaaacfbc

commit a1391f9e9f2528d100ccc1f14f620f3efaaacfbc
Author: David Reveman <reveman@chromium.org>
Date: Fri Sep 08 01:07:07 2017

Change Picture: Improve layout more.

Takes font-size based padding for header into account and also
sets top position for container to ensure that viewport alignment
is still correct even if assumptions about header height are not
perfect.

TBR=reveman@chromium.org

(cherry picked from commit 6a69033c6082f3fc61ffbe7876eb88a83e44d08b)

Bug:  761301 ,  755929 
Test: verified layout is good at all font sizes
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ifbcf8301571fb6313829ef39b1c74eb6926550e3
Reviewed-on: https://chromium-review.googlesource.com/648111
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499767}
Reviewed-on: https://chromium-review.googlesource.com/656699
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#81}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/a1391f9e9f2528d100ccc1f14f620f3efaaacfbc/chrome/browser/resources/settings/people_page/change_picture.html

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 8 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85cd459a0459400a4779184b5d6d2572c49cb1ac

commit 85cd459a0459400a4779184b5d6d2572c49cb1ac
Author: David Reveman <reveman@chromium.org>
Date: Fri Sep 08 01:08:43 2017

Change Picture: Improve layout more.

Takes font-size based padding for header into account and also
sets top position for container to ensure that viewport alignment
is still correct even if assumptions about header height are not
perfect.

TBR=reveman@chromium.org

(cherry picked from commit 6a69033c6082f3fc61ffbe7876eb88a83e44d08b)

Bug:  761301 ,  755929 
Test: verified layout is good at all font sizes
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ifbcf8301571fb6313829ef39b1c74eb6926550e3
Reviewed-on: https://chromium-review.googlesource.com/648111
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499767}
Reviewed-on: https://chromium-review.googlesource.com/656718
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1142}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/85cd459a0459400a4779184b5d6d2572c49cb1ac/chrome/browser/resources/settings/people_page/change_picture.html

Labels: Merge-Approved-63
Labels: -Hotlist-Merge-Review
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 6 2017

Cc: keta...@chromium.org bhthompson@google.com gkihumba@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 14 by sheriffbot@chromium.org, Nov 10 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Sign in to add a comment