Enforce the page title direction

Enforce the page title direction 

WebKit now exposes the direction (in the RTL sense) of page titles.
When we receive a title+direction from WebKit, insert a strong
directionality character in the title to force all downstream
consumers of titles to display the text correctly.

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

[Evan Martin, 2011-07-26 18:59:45]

Hi,

I'd like your advice on this change.  With it, all titles within Chrome will
become guaranteed to being with a strong directionality character, which means
that downstream code no longer needs to guess about directions.

I verified this makes <title dir=rtl> swap the direction of a mixed
english/arabic title.

Dave, how will this affect your recent patches in this area?  (I saw your change
go by but didn't understand it.)

Jeremy, what do you think about this approach?  (This is the one we eventually
agreed upon in various discussions, but just to confirm.)

[DaveMoore, 2011-07-26 23:16:38]

If this guarantees that base::i18n::GetFirstStrongCharacterDirection() returns
RIGHT_TO_LEFT then my new code should work fine.
On 2011/07/26 18:59:45, Evan Martin wrote:
> Hi,
> 
> I'd like your advice on this change.  With it, all titles within Chrome will
> become guaranteed to being with a strong directionality character, which means
> that downstream code no longer needs to guess about directions.
> 
> I verified this makes <title dir=rtl> swap the direction of a mixed
> english/arabic title.
> 
> Dave, how will this affect your recent patches in this area?  (I saw your
change
> go by but didn't understand it.)
> 
> Jeremy, what do you think about this approach?  (This is the one we eventually
> agreed upon in various discussions, but just to confirm.)

[jeremy, 2011-07-27 08:30:37]

LGTM

[brettw, 2011-07-27 22:28:22]

Seems like a fine approach.

http://codereview.chromium.org/7514013/diff/2001/content/renderer/render_view.cc
File content/renderer/render_view.cc (right):

http://codereview.chromium.org/7514013/diff/2001/content/renderer/render_view...
content/renderer/render_view.cc:1189: fixed_title.push_back(0x200E);  // U+200E
LRM
There are consts for these in base/i18n/rtl.h so you don't have to hard code.

[Evan Martin, 2011-07-28 22:25:46]

PTAL

I have a hack in place to not break tests.  I believe the resulting code is
still completely correct in RTL terms; all resulting titles either have explicit
directionality in them or they start with a strongly directional character
already.

[brettw, 2011-07-29 17:14:00]

http://codereview.chromium.org/7514013/diff/9002/content/renderer/render_view.cc
File content/renderer/render_view.cc (right):

http://codereview.chromium.org/7514013/diff/9002/content/renderer/render_view...
content/renderer/render_view.cc:252: bool IsStronglyLTR(char16 ch) {
If the check in rtl.h is slow, it seems like it might be worthwhile to move this
function in there so everybody calling the Wrap function can take advantage of
the fast path.

It might also be nice to name this "Fast" or something since it doesn't actually
do a real check for strong LTR (which would be more complex & slow).

[Evan Martin, 2011-07-30 00:05:14]

PTAL

http://codereview.chromium.org/7514013/diff/9002/content/renderer/render_view.cc
File content/renderer/render_view.cc (right):

http://codereview.chromium.org/7514013/diff/9002/content/renderer/render_view...
content/renderer/render_view.cc:252: bool IsStronglyLTR(char16 ch) {
On 2011/07/29 17:14:00, brettw wrote:
> If the check in rtl.h is slow, it seems like it might be worthwhile to move
this
> function in there so everybody calling the Wrap function can take advantage of
> the fast path.
> 
> It might also be nice to name this "Fast" or something since it doesn't
actually
> do a real check for strong LTR (which would be more complex & slow).

It's more that the function in rtl.h actually scans the string looking for a
strongly directional character, while in this code I'm interested in whether the
*first* character is strongly RTL.

I will try to think of a better name.

[brettw, 2011-08-01 16:16:00]

LGTM

[Evan Martin, 2011-08-01 19:06:53]

It turns out that this doesn't work.  The extensions API exposes the page title,
and we don't want to break extensions that rely on the page title string
equalling the old value.  I think I may have to go back to my previous patches
that pass the title direction around again.  :~(