Fix incorrect forcing of text directionality in canvas_skia.cc.

ui/gfx/canvas_skia.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/canvas.h"
 
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/base/range/range.h"
 #include "ui/base/text/text_elider.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/insets.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/render_text.h"
 #include "ui/gfx/shadow_value.h"
 #include "ui/gfx/skia_util.h"
 
 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
 // any of them are not the halo color, returns true. This defines the halo of
 // pixels that will appear around the text. Note that we have to check each
 // pixel against both the halo color and transparent since |DrawStringWithHalo|
 // will modify the bitmap as it goes, and cleared pixels shouldn't count as
 // changed.
 bool PixelShouldGetHalo(const SkBitmap& bitmap,
                         int x, int y,
@@ skipping 203 matching lines @@
   gfx::Rect clip_rect(text_bounds);
   clip_rect.Inset(ShadowValue::GetMargin(shadows));
 
   canvas_->save(SkCanvas::kClip_SaveFlag);
   ClipRect(clip_rect);
 
   gfx::Rect rect(text_bounds);
   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());
   render_text->SetTextShadows(shadows);
 
   if (flags & MULTI_LINE) {
     ui::WordWrapBehavior wrap_behavior = ui::IGNORE_LONG_WORDS;
     if (flags & CHARACTER_BREAK)
       wrap_behavior = ui::WRAP_LONG_WORDS;
     else if (!(flags & NO_ELLIPSIS))
@@ skipping 120 matching lines @@
 
   // If the whole string fits in the destination then just draw it directly.
   if (GetStringWidth(text, font) <= display_rect.width()) {
     DrawStringInt(text, font, color, display_rect.x(), display_rect.y(),
                   display_rect.width(), display_rect.height(), flags);
     return;
   }
 
   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:
       render_text->set_fade_tail(true);
       if (is_rtl)
         flags |= TEXT_ALIGN_RIGHT;
       break;
     case TruncateFadeHead:
       render_text->set_fade_head(true);
       if (!is_rtl)
@@ skipping 30 matching lines @@
   render_text->SetDisplayRect(rect);
 
   canvas_->save(SkCanvas::kClip_SaveFlag);
   ClipRect(display_rect);
   render_text->Draw(this);
   canvas_->restore();
 }
 #endif
 
 }  // namespace gfx