Fix incorrect forcing of text directionality in canvas_skia.cc.

ui/gfx/canvas_skia.cc

Index: ui/gfx/canvas_skia.cc
===================================================================
--- ui/gfx/canvas_skia.cc	(revision 136322)
+++ ui/gfx/canvas_skia.cc	(working copy)
@@ -19,14 +19,23 @@
 
 namespace {
 
-// Based on |flags| and |text| content, returns whether text should be
-// rendered right-to-left.
-bool IsTextRTL(int flags, const string16& text) {
-  if (flags & gfx::Canvas::FORCE_RTL_DIRECTIONALITY)
-    return true;
-  if (flags & gfx::Canvas::FORCE_LTR_DIRECTIONALITY)
-    return false;
-  return base::i18n::IsRTL() && base::i18n::StringContainsStrongRTLChars(text);
+// If necessary, wraps |text| with RTL/LTR directionality characters based on
+// |flags| and |text| content.
+// Returns true if the text should be rendered right-to-left.
+bool AdjustStringDirection(int flags, string16* text) {
+  bool is_rtl = false;
+  if (flags & gfx::Canvas::FORCE_RTL_DIRECTIONALITY) {
+    base::i18n::WrapStringWithRTLFormatting(text);
+    is_rtl = true;
+  } else if (flags & gfx::Canvas::FORCE_LTR_DIRECTIONALITY) {
+    base::i18n::WrapStringWithLTRFormatting(text);

[xji, 2012/05/14 22:50:51]

This is a bit tricky. In systems without RTL support (such as the he/ar language
package is not installed in Windows Vista), blindly add unicode marker will
render empty squares to displayed.

We had similar problem before when render tooltip.
And what we did last time is that we only add unicode marker when the tooltip's
directionality is not the same as UI's directionality.

I think what we can do here is only WrapStringWithLTRFormatting() when
base::i18n::IsRTL(), assuming the system  has RTL support when UI is RTL or when
text has strong RTL characters in it.

[xji, 2012/05/14 23:09:10]

long term wise, I think we should be able to set text's directionality in
RenderText, instead of adding Unicode bidi marks. Uniscribe and Pango should
support that. It is probably pango_context_set_base_dir().

+    is_rtl = false;
+  } else if (base::i18n::IsRTL() &&
+             base::i18n::StringContainsStrongRTLChars(*text)) {
+    base::i18n::WrapStringWithRTLFormatting(text);
+    is_rtl = true;
+  }
+  return is_rtl;
 }
 
 // Checks each pixel immediately adjacent to the given pixel in the bitmap. If
@@ -250,8 +259,7 @@
   string16 adjusted_text = text;
 
 #if defined(OS_WIN)
-  if (IsTextRTL(flags, adjusted_text))
-    base::i18n::AdjustStringForLocaleDirection(&adjusted_text);
+  AdjustStringDirection(flags, &adjusted_text);
 #endif
 
   scoped_ptr<RenderText> render_text(RenderText::CreateRenderText());
@@ -392,9 +400,7 @@
 
   scoped_ptr<RenderText> render_text(RenderText::CreateRenderText());
   string16 clipped_text = text;
-  const bool is_rtl = IsTextRTL(flags, text);
-  if (is_rtl)
-    base::i18n::AdjustStringForLocaleDirection(&clipped_text);
+  const bool is_rtl = AdjustStringDirection(flags, &clipped_text);
 
   switch (truncate_mode) {
     case TruncateFadeTail: