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

Issue 640907 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

[Mac] Fix profile image icon (and supervised user badge placement) for RTL languages

Reported by dmascare...@etouch.net, Aug 25 2016

Issue description

Version:54.0.2839.0 (Official Build) 911ba12253b14bfe874a321c57031f5ac534ce31-refs/heads/master@{#414243} 32/64 bit
OS: Windows(7,8,10)

Pre-condition: 1. Enable material design user menu flag.
               2. Change the browser language in 'Arabic'

What steps will reproduce the problem?
1. Launch chrome and click on avatar icon,click on 'Switch person' option (switch person overlay gets open).
2. Click on 'Add person' such that new profile gets open and again click on avatar icon, observe.

Actual: Profile image icon seems chopped.
Expected: Profile image icon should be proper.

This is regression issue, broken in 'M 54' and below is narrow bisect:
https://chromium.googlesource.com/chromium/src/+log/8cc17dd5bd16119157decba981d11f1e217052dd..e6fcf92ec826c6b56672eec154e2075b1d01cf60?pretty=fuller&n=100

Suspecting: r406377 

Good build:54.0.2801.0
Bad build:54.0.2802.0

Note: Issue is not seen Mac and Linux OS.

 
actual_profile.png
26.4 KB View Download
Labels: ReleaseBlock-Stable
Adding release block label, please undo if not the case.
This is not RBS as this is behind a flag. Also note that this is happening on both Windows and Linux, but not Mac.

Comment 3 by ew...@chromium.org, Aug 25 2016

Labels: -Pri-1 -ReleaseBlock-Stable md-usermenu signin-active-bug Pri-2

Comment 4 by ew...@chromium.org, Aug 26 2016

Cc: rogerta@chromium.org
Owner: anthonyvd@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30 2016

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

commit bc337a09e7187b86f0fae15e2ff6f66e28246be8
Author: anthonyvd <anthonyvd@chromium.org>
Date: Tue Aug 30 17:34:00 2016

[User Menu] Flipped the profile badge to be on the LHS for RTL layouts

Discussed with bettes@ to decide that the profile badge on RTL layouts
should be on the LHS instead.
Also fixed the position of the circular mask for profile icon on RTL
layouts (see first bug) - the circular mask needs to be bumped to the
right on RTL layouts so that it's right-aligned.

See comparative screenshots:
https://drive.google.com/drive/folders/0B7Fvv7JszRyGN1FKakw4RXE2Vkk?usp=sharing

BUG= 640907 
BUG= 615893 

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

[modify] https://crrev.com/bc337a09e7187b86f0fae15e2ff6f66e28246be8/chrome/browser/ui/views/profiles/profile_chooser_view.cc

Status: Fixed (was: Assigned)

Comment 7 by ew...@chromium.org, Sep 1 2016

Status: Assigned (was: Fixed)
Re-opening to track fixing on Mac.

Comment 8 by ew...@chromium.org, Sep 7 2016

Summary: [Mac] Fix profile image icon (and supervised user badge placement) for RTL languages (was: Regression: Profile image icon seems chopped after adding new profile when browser language changed to 'Arabic'.)

Comment 9 by ew...@chromium.org, Sep 12 2016

Labels: Merge-Request-54 OS-Linux OS-Mac
Requesting a merge for the CL in comment #5 (https://codereview.chromium.org/2285413002). We're still addressing the fix for Mac, but want to merge the fix on Win/Linux.

Comment 10 by dimu@chromium.org, Sep 12 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Could you please confirm whether this change is baked/verified in Canary and safe to merge?If yes, merge your change to M54 (branch: 2840) so that we could take this for next Beta Release.
Project Member

Comment 12 by sheriffbot@chromium.org, Sep 16 2016

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
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 16 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba01d0f8a7b9fa09457807ee6f83401b5b04e18c

commit ba01d0f8a7b9fa09457807ee6f83401b5b04e18c
Author: Anthony Vallee-Dubois <anthonyvd@chromium.org>
Date: Fri Sep 16 15:19:00 2016

[User Menu] Flipped the profile badge to be on the LHS for RTL layouts

Discussed with bettes@ to decide that the profile badge on RTL layouts
should be on the LHS instead.
Also fixed the position of the circular mask for profile icon on RTL
layouts (see first bug) - the circular mask needs to be bumped to the
right on RTL layouts so that it's right-aligned.

See comparative screenshots:
https://drive.google.com/drive/folders/0B7Fvv7JszRyGN1FKakw4RXE2Vkk?usp=sharing

BUG= 640907 
BUG= 615893 

Review-Url: https://codereview.chromium.org/2285413002
Cr-Commit-Position: refs/heads/master@{#415338}
(cherry picked from commit bc337a09e7187b86f0fae15e2ff6f66e28246be8)

Review URL: https://codereview.chromium.org/2345063003 .

Cr-Commit-Position: refs/branch-heads/2840@{#391}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/ba01d0f8a7b9fa09457807ee6f83401b5b04e18c/chrome/browser/ui/views/profiles/profile_chooser_view.cc

Comment 14 by ew...@chromium.org, Sep 19 2016

Labels: -M-54 M-55
Punting the fix for Mac to M55 since it's not launch blocking.
Labels: TE-Verified-M54 TE-Verified-54.0.2840.34
Tested the issue on Chrome Beta# 54.0.2840.34 on Windows, Mac and Linux and is no more reproducible.
Hence adding TE-Verified Labels.

Thank You.
Labels: -M-55 M-56
Cc: anthonyvd@chromium.org
Owner: msarda@chromium.org
After discussing with Anthony:
The remaining issue for this bug is the fact the on macOS the image is not correctly placed on RTL languages.
Labels: TE-Verified-M56 TE-Verified-56.0.2897.0
Tested the issue on Chrome Dev# 56.0.2897.0 on Windows, Mac and Linux and is no more reproducible.
Hence adding TE-Verified Labels.

Thank You.
Owner: anthonyvd@chromium.org
Status: Fixed (was: Assigned)
I take it that this bug is now fixed (it looks like msrchandra@ tested it on macOS as well). I'm thus closing the bug (and reassigning to Anthony as he did all the work).
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 27 2016

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

commit ba01d0f8a7b9fa09457807ee6f83401b5b04e18c
Author: Anthony Vallee-Dubois <anthonyvd@chromium.org>
Date: Fri Sep 16 15:19:00 2016

[User Menu] Flipped the profile badge to be on the LHS for RTL layouts

Discussed with bettes@ to decide that the profile badge on RTL layouts
should be on the LHS instead.
Also fixed the position of the circular mask for profile icon on RTL
layouts (see first bug) - the circular mask needs to be bumped to the
right on RTL layouts so that it's right-aligned.

See comparative screenshots:
https://drive.google.com/drive/folders/0B7Fvv7JszRyGN1FKakw4RXE2Vkk?usp=sharing

BUG= 640907 
BUG= 615893 

Review-Url: https://codereview.chromium.org/2285413002
Cr-Commit-Position: refs/heads/master@{#415338}
(cherry picked from commit bc337a09e7187b86f0fae15e2ff6f66e28246be8)

Review URL: https://codereview.chromium.org/2345063003 .

Cr-Commit-Position: refs/branch-heads/2840@{#391}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/ba01d0f8a7b9fa09457807ee6f83401b5b04e18c/chrome/browser/ui/views/profiles/profile_chooser_view.cc

Sign in to add a comment