Force URL in settings page to be left-to-right.

Force URL in settings page to be left-to-right.

BUG=112169
TEST=launch chrome in Arabic, goto chrome://chrome/settings, under "Appearance->Home page:", url "http://www.google.com/" should be displayed as
"http://www.google.com/", not "/http://www.google.com".

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=125055

[xji, 2012-02-28 22:13:02]

I did not fix "(Enable Instant for faster searching (omnibox input may be
logged" in this CL although I could wrap it into LTR format as well. The reason
is:
1. it used to have a peroid after the sentence. wrapping it into LTR would
render the sentence as "xxxxx." (with period at end) while all other pure
English sentence looks like ".xxxxxx" (with period at begin).
2. It would be better to fix similar errors at one place instead of everywhere.
I am thinking to introduce a flag in DictionarValue to indicate that the string
saved there is for display purpose, and wrap the string in LTR format when the
flag is true.

[Evan Stade, 2012-02-28 22:22:43]

On 2012/02/28 22:13:02, xji wrote:
> I did not fix "(Enable Instant for faster searching (omnibox input may be
> logged" in this CL although I could wrap it into LTR format as well. The
reason
> is:
> 1. it used to have a peroid after the sentence. wrapping it into LTR would
> render the sentence as "xxxxx." (with period at end) while all other pure
> English sentence looks like ".xxxxxx" (with period at begin).
> 2. It would be better to fix similar errors at one place instead of
everywhere.
> I am thinking to introduce a flag in DictionarValue to indicate that the
string
> saved there is for display purpose, and wrap the string in LTR format when the
> flag is true.

I'm confused. The enable instant string should be translated (i.e. will be
translated on google chrome release branches). Why would we wrap that in LTR?

http://codereview.chromium.org/9514015/diff/3/chrome/browser/ui/webui/options...
File chrome/browser/ui/webui/options2/browser_options_handler2.cc (right):

http://codereview.chromium.org/9514015/diff/3/chrome/browser/ui/webui/options...
chrome/browser/ui/webui/options2/browser_options_handler2.cc:551:
IDS_OPTIONS_SHOW_HOME_BUTTON_FOR_URL, home_page);
does it not work to just set dir=ltr on the DOM node?

[xji, 2012-02-28 22:34:11]

On 2012/02/28 22:22:43, Evan Stade wrote:
> On 2012/02/28 22:13:02, xji wrote:
> > I did not fix "(Enable Instant for faster searching (omnibox input may be
> > logged" in this CL although I could wrap it into LTR format as well. The
> reason
> > is:
> > 1. it used to have a peroid after the sentence. wrapping it into LTR would
> > render the sentence as "xxxxx." (with period at end) while all other pure
> > English sentence looks like ".xxxxxx" (with period at begin).
> > 2. It would be better to fix similar errors at one place instead of
> everywhere.
> > I am thinking to introduce a flag in DictionarValue to indicate that the
> string
> > saved there is for display purpose, and wrap the string in LTR format when
the
> > flag is true.
> 
> I'm confused. The enable instant string should be translated (i.e. will be
> translated on google chrome release branches). Why would we wrap that in LTR?
> 

Sorry for the confusion.
The bug will go away after we translate the string.

What I mean is that for all the DOM UI pure English text, if the UI is RTL, we
should wrap the string to LTR format so that the ending punctuation is rendered
correctly.

Or we are not fixing them since they will be translated in release.

http://codereview.chromium.org/9514015/diff/3/chrome/browser/ui/webui/options...
File chrome/browser/ui/webui/options2/browser_options_handler2.cc (right):

http://codereview.chromium.org/9514015/diff/3/chrome/browser/ui/webui/options...
chrome/browser/ui/webui/options2/browser_options_handler2.cc:551:
IDS_OPTIONS_SHOW_HOME_BUTTON_FOR_URL, home_page);
On 2012/02/28 22:22:43, Evan Stade wrote:
> does it not work to just set dir=ltr on the DOM node?

If we translate "show home button for" to Arabic or Hebrew, we should not set
dir=ltr.

[Evan Stade, 2012-03-01 17:47:06]

right. They will be translated for release. No part of our UI should be in
English for a different locale. So this patch isn't necessary.

[xji, 2012-03-01 18:01:54]

On 2012/03/01 17:47:06, Evan Stade wrote:
> right. They will be translated for release. No part of our UI should be in
> English for a different locale. So this patch isn't necessary.

This CL is necessary. Otherwise, when "Show Home Button for" translated into
Arabic or Hebrew as "ABC DEF" and the UI is RTL, the string will be displayed as
"./http://www.google.com FED CBA"

The the part I am talking about to force pure English text to be LTR in RTL UI
is not necessary if we wont fix such issue in non-release version.

[Evan Stade, 2012-03-01 21:59:09]

On 2012/03/01 18:01:54, xji wrote:
> On 2012/03/01 17:47:06, Evan Stade wrote:
> > right. They will be translated for release. No part of our UI should be in
> > English for a different locale. So this patch isn't necessary.
> 
> This CL is necessary. Otherwise, when "Show Home Button for" translated into
> Arabic or Hebrew as "ABC DEF" and the UI is RTL, the string will be displayed
as
> "./http://www.google.com FED CBA"

isn't this correct?

> 
> The the part I am talking about to force pure English text to be LTR in RTL UI
> is not necessary if we wont fix such issue in non-release version.

[Evan Stade, 2012-03-01 21:59:38]

On 2012/03/01 21:59:09, Evan Stade wrote:
> On 2012/03/01 18:01:54, xji wrote:
> > On 2012/03/01 17:47:06, Evan Stade wrote:
> > > right. They will be translated for release. No part of our UI should be in
> > > English for a different locale. So this patch isn't necessary.
> > 
> > This CL is necessary. Otherwise, when "Show Home Button for" translated into
> > Arabic or Hebrew as "ABC DEF" and the UI is RTL, the string will be
displayed
> as
> > "./http://www.google.com FED CBA"
> 
> isn't this correct?

oh, the slash is on the wrong side. So this patch makes it:

".http://www.google.com/ FED CBA" ?

> 
> > 
> > The the part I am talking about to force pure English text to be LTR in RTL
UI
> > is not necessary if we wont fix such issue in non-release version.

[Evan Stade, 2012-03-01 22:01:01]

ok lgtm.

[xji, 2012-03-01 22:58:03]

On 2012/03/01 21:59:38, Evan Stade wrote:
> On 2012/03/01 21:59:09, Evan Stade wrote:
> > On 2012/03/01 18:01:54, xji wrote:
> > > On 2012/03/01 17:47:06, Evan Stade wrote:
> > > > right. They will be translated for release. No part of our UI should be
in
> > > > English for a different locale. So this patch isn't necessary.
> > > 
> > > This CL is necessary. Otherwise, when "Show Home Button for" translated
into
> > > Arabic or Hebrew as "ABC DEF" and the UI is RTL, the string will be
> displayed
> > as
> > > "./http://www.google.com FED CBA"
> > 
> > isn't this correct?
> 
> oh, the slash is on the wrong side. So this patch makes it:
> 
> ".http://www.google.com/ FED CBA" ?
> 

yes.

[commit-bot: I haz the power, 2012-03-01 23:39:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xji@chromium.org/9514015/3

[commit-bot: I haz the power, 2012-03-01 23:39:44]

Can't apply patch for file
chrome/browser/ui/webui/options2/browser_options_handler2.cc.
While running patch -p0 --forward --force;
patching file chrome/browser/ui/webui/options2/browser_options_handler2.cc
Hunk #1 succeeded at 10 (offset 2 lines).
Hunk #2 FAILED at 544.
1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/ui/webui/options2/browser_options_handler2.cc.rej

[xji, 2012-03-02 23:21:31]

PTAL. Setting page changed in r124470 by csilv@. After which, setting "dir=ltr"
in HTML page works fine.

[Evan Stade, 2012-03-05 21:46:37]

awesome. lgtm!

[commit-bot: I haz the power, 2012-03-05 23:00:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xji@chromium.org/9514015/11001

[commit-bot: I haz the power, 2012-03-06 01:03:58]

Change committed as 125055