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

Issue 135639 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Short RTL tab titles are not right-aligned in RTL UI

Project Member Reported by asvitk...@chromium.org, Jul 3 2012

Issue description

Repro steps:

1. Open chrome.exe with --lang=he
2. Go to NTP
3. The tab title should be right aligned.

Current results in Canary and M20 is that it's left-aligned, which is incorrect.

This regressed from Chrome 19, which had the correct behaviour.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 4 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145435

------------------------------------------------------------------------
r145435 | asvitkine@chromium.org | Tue Jul 03 23:13:06 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gfx/canvas_skia.cc?r1=145435&r2=145434&pathrev=145435

Fix default alignment of drawn text when no alignment flags are passed.

This corresponds to logic in canvas_win.cc at the top of ComputeFormatFlags().

BUG= 135639 , 135058 
TEST=Open chrome.exe --lang=he and go to the NTP. Tab title should be right aligned. In the same instance, go to www.google.com. The text "Google" should also be right aligned. There should be no changes in LTR UI.

Review URL: https://chromiumcodereview.appspot.com/10703075
------------------------------------------------------------------------
Labels: Mstone-21 Merge-Requested
Requesting merge to M21 beta. (This is a regression from Chrome M19.)
Labels: -Mstone-21 Mstone-20
Actually, changing merge request to M20 first, since that release is more imminent and this is another RTL regression in M20.

Comment 4 by dharani@google.com, Jul 9 2012

Labels: -Merge-Requested Merge-Approved
Labels: -Merge-Approved
Testing this some more, looks like this regressed drawing long LTR tab titles in RTL UI (since DrawFadeTruncatingString() doesn't always set the alignment).

Withdrawing merge request for now.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 9 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145727

------------------------------------------------------------------------
r145727 | asvitkine@chromium.org | Mon Jul 09 12:51:43 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gfx/canvas_skia.cc?r1=145727&r2=145726&pathrev=145727

Fix fade-truncation of LTR tab titles under RTL UI.

This was regressed by http://crrev.com/145435, which
made UpdateRenderText() get a default alignment based
on the UI directionality in the case where |flags|
didn't specify one.

This caused an issue for DrawFadeTruncatingString(),
which assumed that the default alignment would LTR
unless specified otherwise.

This CL fixes the issue by having DrawFadeTruncatingString()
specify LTR alignment whenever it doesn't explicitly set
it to RTL, which matches its behavior before r145435.

BUG= 135639 
TEST=Open chrome.exe --lang=he and go to amazon.com. The
tab title, which should be in English, should be fade
truncated on the right end. Go to http://www.haaretz.co.il
and text should be fade truncated on the left end. When
launched in LTR mode (chrome.exe --lang=en), both of those
tab titles should be fade-truncated on right, per the M19
behavior.

Review URL: https://chromiumcodereview.appspot.com/10743002
------------------------------------------------------------------------
Okay, should be all good now. Will wait till the latest CL hits Canary to verify that everything looks good and will ask again for a merge (of both CLs), assuming everything is good.
Labels: Merge-Requested
Verified with current Canary (22.0.1202.0) that the two fixes are working correctly.

Re-requesting merges of r145435 and r145727 to M20.
Labels: -Merge-Requested Merge-Approved
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 10 2012

Labels: merge-merged-1132
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145917

------------------------------------------------------------------------
r145917 | asvitkine@chromium.org | Tue Jul 10 11:09:29 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1132/src/ui/gfx/canvas_skia.cc?r1=145917&r2=145916&pathrev=145917

Merge 145435 - Fix default alignment of drawn text when no alignment flags are passed.

This corresponds to logic in canvas_win.cc at the top of ComputeFormatFlags().

BUG= 135639 , 135058 
TEST=Open chrome.exe --lang=he and go to the NTP. Tab title should be right aligned. In the same instance, go to www.google.com. The text "Google" should also be right aligned. There should be no changes in LTR UI.

Review URL: https://chromiumcodereview.appspot.com/10703075

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10700143
------------------------------------------------------------------------
Labels: -Mstone-20 -Merge-Approved Mstone-21 Merge-Requested
Merged to M20, although I'm told these missed the train for the next M20 push.

Also requesting merge to M21 beta branch.
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 10 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145918

------------------------------------------------------------------------
r145918 | asvitkine@chromium.org | Tue Jul 10 11:10:16 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1132/src/ui/gfx/canvas_skia.cc?r1=145918&r2=145917&pathrev=145918

Merge 145727 - Fix fade-truncation of LTR tab titles under RTL UI.

This was regressed by http://crrev.com/145435, which
made UpdateRenderText() get a default alignment based
on the UI directionality in the case where |flags|
didn't specify one.

This caused an issue for DrawFadeTruncatingString(),
which assumed that the default alignment would LTR
unless specified otherwise.

This CL fixes the issue by having DrawFadeTruncatingString()
specify LTR alignment whenever it doesn't explicitly set
it to RTL, which matches its behavior before r145435.

BUG= 135639 
TEST=Open chrome.exe --lang=he and go to amazon.com. The
tab title, which should be in English, should be fade
truncated on the right end. Go to http://www.haaretz.co.il
and text should be fade truncated on the left end. When
launched in LTR mode (chrome.exe --lang=en), both of those
tab titles should be fade-truncated on right, per the M19
behavior.

Review URL: https://chromiumcodereview.appspot.com/10743002

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10700144
------------------------------------------------------------------------

Comment 13 by kareng@google.com, Jul 10 2012

Labels: -Merge-Requested Merge-Approved
approved for m21.
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 10 2012

Labels: -Merge-Approved merge-merged-1180
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145941

------------------------------------------------------------------------
r145941 | asvitkine@chromium.org | Tue Jul 10 12:58:12 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/ui/gfx/canvas_skia.cc?r1=145941&r2=145940&pathrev=145941

Merge 145435 - Fix default alignment of drawn text when no alignment flags are passed.

This corresponds to logic in canvas_win.cc at the top of ComputeFormatFlags().

BUG= 135639 , 135058 
TEST=Open chrome.exe --lang=he and go to the NTP. Tab title should be right aligned. In the same instance, go to www.google.com. The text "Google" should also be right aligned. There should be no changes in LTR UI.

Review URL: https://chromiumcodereview.appspot.com/10703075

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10703128
------------------------------------------------------------------------
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 10 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=145942

------------------------------------------------------------------------
r145942 | asvitkine@chromium.org | Tue Jul 10 12:58:50 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/ui/gfx/canvas_skia.cc?r1=145942&r2=145941&pathrev=145942

Merge 145727 - Fix fade-truncation of LTR tab titles under RTL UI.

This was regressed by http://crrev.com/145435, which
made UpdateRenderText() get a default alignment based
on the UI directionality in the case where |flags|
didn't specify one.

This caused an issue for DrawFadeTruncatingString(),
which assumed that the default alignment would LTR
unless specified otherwise.

This CL fixes the issue by having DrawFadeTruncatingString()
specify LTR alignment whenever it doesn't explicitly set
it to RTL, which matches its behavior before r145435.

BUG= 135639 
TEST=Open chrome.exe --lang=he and go to amazon.com. The
tab title, which should be in English, should be fade
truncated on the right end. Go to http://www.haaretz.co.il
and text should be fade truncated on the left end. When
launched in LTR mode (chrome.exe --lang=en), both of those
tab titles should be fade-truncated on right, per the M19
behavior.

Review URL: https://chromiumcodereview.appspot.com/10743002

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10692139
------------------------------------------------------------------------
Status: Fixed
All fixed and merged.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 9 2013

Labels: -Type-Regression -Area-UI -Mstone-21 Type-Bug-Regression Cr-UI M-21
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 14 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment