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

Issue 134759 link

Starred by 2 users

Issue metadata

Status: Verified
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

REGRESSION: RTL Chrome UI fades the left/leading side of LTR tab titles.

Project Member Reported by msw@chromium.org, Jun 26 2012

Issue description

Version: 22.0.1187.0 (גירסה רשמית 144126) canary
OS: Win7, maybe more?

What steps will reproduce the problem?
1. Run chrome with a RTL UI language.
2. Visit a site with a long LTR tab title, like amazon.com.

Expected: The tab title fades and truncates on the right side.
Actual: The tab title fades on the left side, not the right?
Note that it correctly truncates the right/trailing side of the title.
It's odd that one side fades and the other side truncates without fading.

Note that RTL strings, like that for "New Tab" behave correctly.
See the attached picture for examples of both of these cases.

I'm not sure about RTL titles in LTR UI, or BiDi strings in either.
 
rtl_tab_fade.png
19.0 KB View Download
This happens because RenderText::ApplyFadeDirection() swaps fade location based on RenderText::GetTextDirection(), which for RenderTextWin returns RIGHT_TO_LEFT based on base::i18n::IsRTL().

Comment 2 by msw@chromium.org, Jun 27 2012

Thanks for checking that; how does it know to left-align the text?
This may be fixed with  Issue 134746  if we use first strong directional char.

The align logic is done at the canvas_skia.cc level and set explicitly.
Status: Started
CL here: http://codereview.chromium.org/10689013/

This an M20 regression (which is going to stable this week :( ), so we'll probably want to merge this back.


Comment 5 by msw@chromium.org, Jun 27 2012

So this worked correctly in M19?
Your fix needs testing to avoid further regressions before considering merge.
For sure. It should land on Canary first.
(It regressed in M20 because M20 switched use_canvas_skia=1 on. M19 still used canvas_win.cc.)

Comment 8 by msw@chromium.org, Jun 27 2012

Gotcha, thanks for being on top of this and the quick fix.
Cc: kareng@google.com dharani@chromium.org vivianz@chromium.org
Karen/Dharani: Heads up - this is a pretty visible regression in M20 under RTL locales, so I'll likely request merging to M21 and possibly M20 once this bakes in Canary.

Vivian: Would it be possible to get a bit of manual QA coverage of RTL/BiDi tab title fading on Windows once this fix lands? (It's in the commit queue right now and so will likely hit tomorrow Canary.)
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 28 2012

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

------------------------------------------------------------------------
r144791 | asvitkine@chromium.org | Thu Jun 28 13:30:36 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gfx/render_text.cc?r1=144791&r2=144790&pathrev=144791

Fix tab title fading direction in RTL locales.

BUG= 134759 ,  134746 
TEST=On Windows, launch chrome.exe with --lang=he and go to http://en.wikipedia.org/wiki/Main_Page. Observe that the text is faded on the right.
Open another tab and go to http://www.bbc.co.uk/arabic/topics/syria/ and observe that the text is faded on the left. Observe the same is true with LTR locales.

Review URL: https://chromiumcodereview.appspot.com/10689013
------------------------------------------------------------------------
The fix is now in today's Canary - 22.0.1190.0.

I can confirm that the issue is fixed per the steps described in the repro steps in comment 10 and the original description.
Labels: -Type-Bug -Pri-2 Type-Regression Pri-1 Mstone-21 Merge-Requested
Summary: REGRESSION: RTL Chrome UI fades the left/leading side of LTR tab titles.
Putting labels to request merge to M21 to get the attention of the TPMs.

This is a pretty visible regression that took place in M20 which causes incorrect rendering of all English (and other LTR) tab titles when your UI is set to RTL (e.g. Hebrew).

(I'll also would like to try to get this merged to M20 - but let's start with M21 for now.)

Comment 13 by kareng@google.com, Jun 29 2012

can u please also check crashes to make srue the fix didn't introduce a new crash? 
I'll check, but the fix is so simple (just changing a one line if statement condition that swaps the fade direction) that it should be very very safe.
I've checked:

http://chromecrash/browse?q=product.name%3D'Chrome'%20AND%20product.version%3D'22.0.1190.0'%20AND%20custom_data.ChromeCrashProto.ptype%3D'browser'

and clicked the "top 1000" link for the first table (which showed 29 groups of crashes). None of those look like they could be caused by this change.

Comment 16 by kareng@google.com, Jun 29 2012

Labels: -Merge-Requested Merge-Approved
sg ty for confirming. 
Project Member

Comment 17 by bugdroid1@chromium.org, Jun 29 2012

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

------------------------------------------------------------------------
r144983 | asvitkine@chromium.org | Fri Jun 29 13:55:42 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1180/src/ui/gfx/render_text.cc?r1=144983&r2=144982&pathrev=144983

Merge 144791 - Fix tab title fading direction in RTL locales.

BUG= 134759 ,  134746 
TEST=On Windows, launch chrome.exe with --lang=he and go to http://en.wikipedia.org/wiki/Main_Page. Observe that the text is faded on the right.
Open another tab and go to http://www.bbc.co.uk/arabic/topics/syria/ and observe that the text is faded on the left. Observe the same is true with LTR locales.

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

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10702047
------------------------------------------------------------------------
Labels: -Mstone-21 Mstone-20 Merge-Requested
Also requesting merge to M20 since this is a regression from M19.

Comment 19 by msw@chromium.org, Jul 3 2012

AFAICT on LTR locale ToT/Canary, pure RTL tab titles fade/truncate on the right.
That seems like a bug and conflicts with your TEST= note for http://crrev.com/144791
Note that example in your TEST= is LTR-RTL, try http://www.haaretz.co.il instead.
Labels: -Merge-Requested
Removing merge request label until I can investigate the issue msw is seeing.
I just checked Chrome 19 (that uses the old canvas_win.cc path), and it behaves the same as Canary on http://www.haaretz.co.il when using LTR UI.

So the issue raised in comment 19 is not a regression, but behaves as expected (i.e. as previous versions). If we want to change that behaviour, that should be tracked under a separate bug.

Note: I believe the reason it is currently the way it is (and xji@ can correct me if I'm wrong), is because forcing directionality is currently done by wrapping with Unicode BiDi control chars - and we don't want to insert those for non-RTL UIs as they may show up as boxes on machines without i18n packs installed.
Labels: Merge-Requested
I just re-tested and am seeing current Canary that has the fix behaving the same way as Chrome 19 (pre-regression).

So putting back merge request label to merge this regression fix to M20.
I've tested more thoroughly now and there is indeed another issue in both TOT and M20 (unaffected by this fix), with the horizontal alignment of tab titles in RTL UI.

RTL titles in RTL UI are not right-aligned if they are short enough to be elided. This is a separate issue from this bug and unaffected by this fix; I've filed it as:  http://crbug.com/135639 

Labels: -Merge-Requested Merge-Approved
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 3 2012

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

------------------------------------------------------------------------
r145353 | asvitkine@chromium.org | Tue Jul 03 12:19:28 PDT 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1132/src/ui/gfx/render_text.cc?r1=145353&r2=145352&pathrev=145353

Merge 144791 - Fix tab title fading direction in RTL locales.

BUG= 134759 ,  134746 
TEST=On Windows, launch chrome.exe with --lang=he and go to http://en.wikipedia.org/wiki/Main_Page. Observe that the text is faded on the right.
Open another tab and go to http://www.bbc.co.uk/arabic/topics/syria/ and observe that the text is faded on the left. Observe the same is true with LTR locales.

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

TBR=asvitkine@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10694073
------------------------------------------------------------------------
Status: Fixed
Marking as Fixed as this has now landed on TOT and has been merged to M21 and M20.
Status: Verified
Verified with Chrome 20.0.1132.57 (Official build 145807)/Win7

Comment 28 by k...@google.com, Aug 8 2012

Labels: -Merge-Approved
Remove merge approval label, this release has passed.

Comment 29 by k...@google.com, Aug 8 2012

Remove merge approval label, this release has passed.
Project Member

Comment 30 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 31 by bugdroid1@chromium.org, Mar 9 2013

Labels: -Type-Regression -Area-UI -Feature-TabStrip -Feature-I18N -Internals-Views -Mstone-20 Type-Bug-Regression Cr-UI-Browser-TabStrip Cr-UI M-20 Cr-UI-I18N Cr-Internals-Views
Project Member

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

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 20 2013

Labels: -Cr-UI-I18N Cr-UI-Internationalization

Sign in to add a comment