New issue
Advanced search Search tips

Issue 850390 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Regression
Team-Security-UX



Sign in to add a comment

Body text on page info bubble is left-aligned in RTL

Project Member Reported by tapted@chromium.org, Jun 7 2018

Issue description

Chrome Version       : 68.0.3440.1

What steps will reproduce the problem?
1. chrome://flags/#force-ui-direction set to right-to-left or switch to an RTL language
2. Open a site, click padlock

What is the expected result?

right-aligned body text

What happens instead of that?

body text is left-aligned

Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.1 Safari/537.36



 
artl.png
15.8 KB View Download
Labels: Needs-Triage-M68
Cc: viswa.karala@chromium.org
Labels: -Type-Bug -Pri-3 Target-67 RegressedIn-66 M-69 hasbisect FoundIn-67 Target-69 Triaged-ET Target-68 FoundIn-68 FoundIn-69 OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: wutao@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on the reported chrome 68.0.3440.1,latest canary 69.0.3453.0 also on latest stable 67.0.3396.79 using Windows10,Mac OS10.13.3 ,Ubuntu14.04.Below is the bisect information for same.

Bisect Info:
================
Good build: 66.0.3336.0
Bad build: 66.0.3336.0

You are probably looking for a change made after 534186 (known good), but no later than 534200 (first known bad).

CHANGELOG URL:
https://chromium.googlesource.com/chromium/src/+log/5685d4cddd5a7e36bb837a483169a88971163208..dd394d24f61f8969cdfaaa338bcac9511744de30

Reviewed-on:  https://chromium-review.googlesource.com/888543

Suspecting same from change log.

@wutao:Please confirm the issue and help in re-assigning if it is not related to your change.

Thanks!

Comment 3 by wutao@chromium.org, Jun 8 2018

Cc: msw@chromium.org
+msw@, FYI.

Comment 4 by wutao@chromium.org, Jun 8 2018

StyledLable is using base::i18n::IsRTL() to determine RTL [1], which is using  GetTextDirectionForLocaleInStartUp() [2], which calss GetForcedTextDirection() uses the flag force-ui-direction [3].


[1] https://cs.chromium.org/chromium/src/base/i18n/rtl.cc?l=153&rcl=401867afab6dfa267f75d127bc047c6e62f6899c

[2] https://cs.chromium.org/chromium/src/base/i18n/rtl.cc?l=187&rcl=401867afab6dfa267f75d127bc047c6e62f6899c

[3] https://cs.chromium.org/chromium/src/base/i18n/rtl.cc?l=165&rcl=401867afab6dfa267f75d127bc047c6e62f6899c

Comment 6 by wutao@chromium.org, Jun 8 2018

msw@, I do not see we change the direction of text in our StyledLable cl mentioned in the bug. It is handled by underlining class Label.cc.

Comment 7 by msw@chromium.org, Jun 8 2018

Perhaps the logic in StyledLabel::AdvanceOneLine() or HorizontalAdjustment() is wrong:
https://cs.chromium.org/chromium/src/ui/views/controls/styled_label.cc?rcl=4456afbc261e3dd74d3e764a4de7a90aa9703a82&l=537
If you add a border to Label to paint its bounds visually, that should reveal if the Label itself is handling internal alignment incorrectly.

Comment 8 by msw@chromium.org, Jun 8 2018

It's also possible that StyledLabel needs to set the alignment of the Labels that it creates.

Comment 9 by wutao@chromium.org, Jun 9 2018

Labels: OS-Android OS-Chrome OS-Fuchsia OS-iOS
Status: Started (was: Assigned)
I was confused this bug, it reports the body text alignment, not the text direction.

The default alignment of label was wrong. It should be always ALIGN_LEFT
no matter it is RTL or LTR. The default behavior is to align left in LTR
UI and right in RTL UI. 

Uploaded a cl to: crrev.com/c/1094328
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 12 2018

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

commit 51b82ab377fcaa288ac00464ebfc2398971109ca
Author: wutao <wutao@chromium.org>
Date: Tue Jun 12 21:19:49 2018

StyledLabel: Fix default alignment in RTL

The default alignment of label was wrong. It should be always ALIGN_LEFT
no matter it is RTL or LTR. The default behavior is to align left in LTR
UI and right in RTL UI.

Bug:  850390 
Test: add new unittest: StyledLabelTest.*AlignmentInRTL*
Change-Id: I058afab144acd46e221f889c57207340ef5aa26e
Reviewed-on: https://chromium-review.googlesource.com/1094328
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566573}
[modify] https://crrev.com/51b82ab377fcaa288ac00464ebfc2398971109ca/ui/views/controls/styled_label.cc
[modify] https://crrev.com/51b82ab377fcaa288ac00464ebfc2398971109ca/ui/views/controls/styled_label.h
[modify] https://crrev.com/51b82ab377fcaa288ac00464ebfc2398971109ca/ui/views/controls/styled_label_unittest.cc

Comment 11 by wutao@chromium.org, Jun 12 2018

Labels: Merge-Request-68
Status: Fixed (was: Started)
Request merge to M68.

M67 is already stable, maybe it is too late to merge to M67.
Project Member

Comment 12 by sheriffbot@chromium.org, Jun 12 2018

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: TE-Verified-M69 TE-Verified-69.0.3457.0
Able to reproduce the issue on chrome version 67.0.3396.79
Verified the fix on Mac 10.12.6, Windows-10 & Ubuntu 14.04 on Chrome version #69.0.3457.0 as per the comment#0
Attaching screenshot for reference.
Observed "right-aligned body text"
Hence, the fix is working as expected.
Adding the verified label.

Thanks!
850390.png
329 KB View Download
Labels: -Merge-Review-68 Merge-Approved-68
Labels: -Hotlist-Merge-Review
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 13 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d27fe8c4bc6ade806dc5fe495dc02c81598fad18

commit d27fe8c4bc6ade806dc5fe495dc02c81598fad18
Author: wutao <wutao@chromium.org>
Date: Wed Jun 13 19:54:08 2018

M68 merge: StyledLabel: Fix default alignment in RTL

The default alignment of label was wrong. It should be always ALIGN_LEFT
no matter it is RTL or LTR. The default behavior is to align left in LTR
UI and right in RTL UI.

TBR=msw@chromium.org

Bug:  850390 
Test: add new unittest: StyledLabelTest.*AlignmentInRTL*
Change-Id: I058afab144acd46e221f889c57207340ef5aa26e
Reviewed-on: https://chromium-review.googlesource.com/1094328
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#566573}(cherry picked from commit 51b82ab377fcaa288ac00464ebfc2398971109ca)
Reviewed-on: https://chromium-review.googlesource.com/1099696
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#342}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/d27fe8c4bc6ade806dc5fe495dc02c81598fad18/ui/views/controls/styled_label.cc
[modify] https://crrev.com/d27fe8c4bc6ade806dc5fe495dc02c81598fad18/ui/views/controls/styled_label.h
[modify] https://crrev.com/d27fe8c4bc6ade806dc5fe495dc02c81598fad18/ui/views/controls/styled_label_unittest.cc

Comment 17 by wutao@chromium.org, Jun 13 2018

Thank you viswa.karala@ and cmasso@.

Sign in to add a comment