===================================================

This change list fixes the following 2 chrome bugs:

2821 RTL: Ellipsis appeared at right for RTL locales on "Task Manager - Google Chrome" dialog
(http://crbug.com/2821)

6132: Strings in tables are displayed LTR when they should be RTL
(http://crbug.com/6132)

===================================================
Background:

The above 2 bugs are related to Chrome task manager.
Task manager is a TableView, which is a subclass of NativeControl.

Currently, TableView window is created by using WS_EX_LAYOUTRTL | WS_EX_RTLREADING flag, which is wrong.

WS_EX_LAYOUTRTL | WS_EX_RTLREADING renders a RTL alignment but LTR reading order.
Please see the following blog for detail:
http://blogs.msdn.com/michkap/archive/2007/03/11/1857043.aspx

We need to replace the above flag with WS_EX_LAYOUTRTL only.

Consequently, we need to make sure that the various implementations of TableModel::GetText adjust the 
returned text appropriately if need be. For example, if the text in an URL, then the returned text should be explicitly marked as LTR text.

Please note: there are several other places where we create RTL window using WS_EX_LAYOUTRTL | WS_EX_RTLREADING flag, such as AutoCompleteEditView (omnibox), MenuHostWindow,
TextField (for example, text in bookmark dialog), and other NativeControl (including checkbox, combobox, native_button, radio_button, separator, tabbed_pane, and tree view).

They all need to be fixed. And the following bug is filed for tracking.
http://crbug.com/6573
 
===================================================

The fix is based on Idan's comments on bug 6132

1.create the table view with WS_EX_LAYOUTRTL (rather than WS_EX_LAYOUTRTL | WS_EX_RTLREADING) 

2 Make sure that the various implementations of TableModel::GetText adjust the returned text appropriately if need be. 
If the text in an URL, then the returned text is explicitly marked as LTR text. 
Otherwise, the returned text is marked with LTR if it is RTL environment and there is no strong RTL character in the text. This is to avoid the wrong placement of (or wrong mirrored) ending-punctuation.

===================================================
Since we have quite a few places creating window using WS_EX_LAYOUTRTL | WS_EX_RTLREADING style. We decided to migrate to the right style step to step.

This is the first step, and there is unnecessary function introduced by this change list.

1.
I am using l10n_util::GetExtendedTooltipStyles() for the right style for now. And I marked a TODO there to obsolete this function name, replace with GetExtendedStyles eventually. (the l10n_util.h is no longer in the change list since I've checked it in with another CL).

2. 
I introduced native_control::GetAdditionalRTLStyle() which should replace GetAdditionalExStyle() eventually.


===================================================
When wrapping text with LTR format in GetText(),
ideally, we should parse the text to check whether this is a n URL or not (for example, a webpage title could be an URL, or user could add URL as search engine's name or keyword).
I did not do that and only added a TODO there because I think that might be rare case and parsing the text might cause performance degradation.

Review URL: http://codereview.chromium.org/18076

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8593 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 154 insertions, 36 deletions

diff --git a/chrome/browser/bookmarks/bookmark_table_model.cc b/chrome/browser/bookmarks/bookmark_table_model.cc
index 2b31c72..c175217 100644
--- a/chrome/browser/bookmarks/bookmark_table_model.cc
+++ b/chrome/browser/bookmarks/bookmark_table_model.cc
@@ -275,16 +275,38 @@ BookmarkTableModel::~BookmarkTableModel() {
 std::wstring BookmarkTableModel::GetText(int row, int column_id) {
   BookmarkNode* node = GetNodeForRow(row);
   switch (column_id) {
-    case IDS_BOOKMARK_TABLE_TITLE:
-      return node->GetTitle();
+    case IDS_BOOKMARK_TABLE_TITLE: {
+      std::wstring title = node->GetTitle();
+      // Adjust the text as well, for example, put LRE-PDF pair around LTR text
+      // in RTL enviroment, so that the ending punctuation in the text will not
+      // be rendered incorrectly (such as rendered as the leftmost character,
+      // and/or rendered as a mirrored punctuation character).
+      //
+      // TODO(xji): Consider adding a special case if the title text is a URL,
+      // since those should always be displayed LTR. Please refer to
+      // http://crbug.com/6726 for more information.
+      l10n_util::AdjustStringForLocaleDirection(title, &title);
+      return title;
+    }
 
-    case IDS_BOOKMARK_TABLE_URL:
-      return node->is_url() ? UTF8ToWide(node->GetURL().spec()) :
-          std::wstring();
+    case IDS_BOOKMARK_TABLE_URL: {
+      if (!node->is_url())
+        return std::wstring();
+      std::wstring url_text = UTF8ToWide(node->GetURL().spec());
+      if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+        l10n_util::WrapStringWithLTRFormatting(&url_text);
+      return url_text;
+    }
 
     case IDS_BOOKMARK_TABLE_PATH: {
       std::wstring path;
       BuildPath(node->GetParent(), &path);
+      // Force path to have LTR directionality. The whole path needs to be
+      // marked with LRE-PDF, so that the path containing both LTR and RTL
+      // subfolder names (such as "CBA/FED/(xji)/.x.j.", in which, "CBA" and
+      // "FED" are subfolder names in Hebrew) can be displayed as LTR.
+      if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+        l10n_util::WrapStringWithLTRFormatting(&path);
       return path;
     }
   }
@@ -336,5 +358,11 @@ void BookmarkTableModel::BuildPath(BookmarkNode* node, std::wstring* path) {
   }
   BuildPath(node->GetParent(), path);
   path->append(l10n_util::GetString(IDS_BOOKMARK_TABLE_PATH_SEPARATOR));
+  // Force path to have LTR directionality. Otherwise, folder path "CBA/FED"
+  // (in which, "CBA" and "FED" stand for folder names in Hebrew, and "FED" is
+  // a subfolder of "CBA") will be displayed as "FED/CBA".
+  if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
+    path->append(l10n_util::kLeftToRightMark);
+  }
   path->append(node->GetTitle());
 }
diff --git a/chrome/browser/task_manager.cc b/chrome/browser/task_manager.cc
index 878c734..14a3e14 100644
--- a/chrome/browser/task_manager.cc
+++ b/chrome/browser/task_manager.cc
@@ -114,7 +114,12 @@ std::wstring TaskManagerTableModel::GetText(int row, int col_id) {
         return l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT);
       if (net_usage == 0)
         return std::wstring(L"0");
-      return FormatSpeed(net_usage, GetByteDisplayUnits(net_usage), true);
+      std::wstring net_byte =
+          FormatSpeed(net_usage, GetByteDisplayUnits(net_usage), true);
+      // Force number string to have LTR directionality.
+      if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+        l10n_util::WrapStringWithLTRFormatting(&net_byte);
+      return net_byte;
     }
 
     case IDS_TASK_MANAGER_CPU_COLUMN:  // CPU
@@ -122,28 +127,28 @@ std::wstring TaskManagerTableModel::GetText(int row, int col_id) {
         return std::wstring();
       return IntToWString(GetCPUUsage(resource));
 
-    case IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN:  // Memory
+    case IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN: {  // Memory
       // We report committed (working set + paged) private usage. This is NOT
       // going to match what Windows Task Manager shows (which is working set).
       if (!first_in_group)
         return std::wstring();
-      return l10n_util::GetStringF(
-          IDS_TASK_MANAGER_MEM_CELL_TEXT,
-          FormatNumber(GetPrivateMemory(process_metrics)));
+      std::wstring number = FormatNumber(GetPrivateMemory(process_metrics));
+      return GetMemCellText(&number);
+    }
 
-    case IDS_TASK_MANAGER_SHARED_MEM_COLUMN:  // Memory
+    case IDS_TASK_MANAGER_SHARED_MEM_COLUMN: {  // Memory
       if (!first_in_group)
         return std::wstring();
-      return l10n_util::GetStringF(
-          IDS_TASK_MANAGER_MEM_CELL_TEXT,
-          FormatNumber(GetSharedMemory(process_metrics)));      
+      std::wstring number = FormatNumber(GetSharedMemory(process_metrics));
+      return GetMemCellText(&number);
+    }
 
-    case IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN:  // Memory
+    case IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN: {  // Memory
       if (!first_in_group)
         return std::wstring();
-      return l10n_util::GetStringF(
-          IDS_TASK_MANAGER_MEM_CELL_TEXT,
-          FormatNumber(GetPhysicalMemory(process_metrics)));
+      std::wstring number = FormatNumber(GetPhysicalMemory(process_metrics));
+      return GetMemCellText(&number);
+    }
 
     case IDS_TASK_MANAGER_PROCESS_ID_COLUMN:
       if (!first_in_group)
@@ -212,6 +217,13 @@ int TaskManagerTableModel::GetStatsValue(TaskManager::Resource* resource,
   return 0;
 }
 
+std::wstring TaskManagerTableModel::GetMemCellText(
+    std::wstring* number) const {
+  // Adjust number string if necessary.
+  l10n_util::AdjustStringForLocaleDirection(*number, number);
+  return l10n_util::GetStringF(IDS_TASK_MANAGER_MEM_CELL_TEXT, *number);
+}
+
 SkBitmap TaskManagerTableModel::GetIcon(int row) {
   DCHECK(row < RowCount());
   SkBitmap icon = resources_[row]->GetIcon();
diff --git a/chrome/browser/task_manager.h b/chrome/browser/task_manager.h
index 90111fc..7c6602b 100644
--- a/chrome/browser/task_manager.h
+++ b/chrome/browser/task_manager.h
@@ -267,7 +267,7 @@ class TaskManagerTableModel : public views::GroupTableModel,
   // Returns the stat value at the column |col_id| that should be displayed from
   // the passed |process_metrics|.
   int GetStatsValue(TaskManager::Resource* resource, int col_id);
-  
+
   // Retrieves the ProcessMetrics for the resources at the specified rows.
   // Returns true if there was a ProcessMetrics available for both rows.
   bool GetProcessMetricsForRows(int row1,
@@ -275,6 +275,10 @@ class TaskManagerTableModel : public views::GroupTableModel,
                                 base::ProcessMetrics** proc_metrics1,
                                 base::ProcessMetrics** proc_metrics2);
 
+  // Given a string containing a number, this function returns the formatted
+  // string that should be displayed in the task manager's memory cell.
+  std::wstring GetMemCellText(std::wstring* number) const;
+
   // The list of providers to the task manager. They are ref counted.
   ResourceProviderList providers_;
 
diff --git a/chrome/browser/task_manager_resource_providers.cc b/chrome/browser/task_manager_resource_providers.cc
index 50b55ac..8576471 100644
--- a/chrome/browser/task_manager_resource_providers.cc
+++ b/chrome/browser/task_manager_resource_providers.cc
@@ -48,8 +48,23 @@ std::wstring TaskManagerWebContentsResource::GetTitle() const {
 
   // Fall back on the URL if there's no title.
   std::wstring tab_title(web_contents_->GetTitle());
-  if (tab_title.empty())
+  if (tab_title.empty()) {
     tab_title = UTF8ToWide(web_contents_->GetURL().spec());
+    // Force URL to be LTR.
+    if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+      l10n_util::WrapStringWithLTRFormatting(&tab_title);
+  } else {
+    // Since the tab_title will be concatenated with
+    // IDS_TASK_MANAGER_TAB_PREFIX, we need to explicitly set the tab_title to
+    // be LTR format if there is no strong RTL charater in it. Otherwise, if
+    // IDS_TASK_MANAGER_TAB_PREFIX is an RTL word, the concatenated result
+    // might be wrong. For example, http://mail.yahoo.com, whose title is
+    // "Yahoo! Mail: The best web-based Email!", without setting it explicitly
+    // as LTR format, the concatenated result will be "!Yahoo! Mail: The best
+    // web-based Email :BAT", in which the capital letters "BAT" stands for
+    // the Hebrew word for "tab".
+    l10n_util::AdjustStringForLocaleDirection(tab_title, &tab_title);
+  }
 
   return l10n_util::GetStringF(IDS_TASK_MANAGER_TAB_PREFIX, tab_title);
 }
@@ -253,6 +268,11 @@ std::wstring TaskManagerPluginProcessResource::GetTitle() const {
       plugin_name = info.name;
     else
       plugin_name = l10n_util::GetString(IDS_TASK_MANAGER_UNKNOWN_PLUGIN_NAME);
+    // Explicitly mark plugin_name as LTR if there is no strong RTL character,
+    // to avoid the wrong concatenation result similar to "!Yahoo! Mail: the
+    // best web-based Email: NIGULP", in which "NIGULP" stands for the Hebrew
+    // or Arabic word for "plugin".
+    l10n_util::AdjustStringForLocaleDirection(plugin_name, &plugin_name);
     title_ = l10n_util::GetStringF(IDS_TASK_MANAGER_PLUGIN_PREFIX,
                                    plugin_name);
   }
diff --git a/chrome/browser/views/hung_renderer_view.cc b/chrome/browser/views/hung_renderer_view.cc
index bf35e50..86b2d6b 100644
--- a/chrome/browser/views/hung_renderer_view.cc
+++ b/chrome/browser/views/hung_renderer_view.cc
@@ -85,6 +85,10 @@ std::wstring HungPagesTableModel::GetText(int row, int column_id) {
   std::wstring title = webcontentses_.at(row)->GetTitle();
   if (title.empty())
     title = l10n_util::GetString(IDS_TAB_UNTITLED_TITLE);
+  // TODO(xji): Consider adding a special case if the title text is a URL,
+  // since those should always have LTR directionality. Please refer to
+  // http://crbug.com/6726 for more information.
+  l10n_util::AdjustStringForLocaleDirection(title, &title);
   return title;
 }
 
diff --git a/chrome/browser/views/keyword_editor_view.cc b/chrome/browser/views/keyword_editor_view.cc
index 62063df..986ce09 100644
--- a/chrome/browser/views/keyword_editor_view.cc
+++ b/chrome/browser/views/keyword_editor_view.cc
@@ -191,15 +191,29 @@ std::wstring TemplateURLTableModel::GetText(int row, int col_id) {
   const TemplateURL& url = entries_[row]->template_url();
 
   switch (col_id) {
-    case IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN:
+    case IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN: {
+      std::wstring url_short_name = url.short_name();
+      // TODO(xji): Consider adding a special case if the short name is a URL,
+      // since those should always be displayed LTR. Please refer to
+      // http://crbug.com/6726 for more information.
+      l10n_util::AdjustStringForLocaleDirection(url_short_name,
+                                                &url_short_name);
       return (template_url_model_->GetDefaultSearchProvider() == &url) ?
           l10n_util::GetStringF(IDS_SEARCH_ENGINES_EDITOR_DEFAULT_ENGINE,
-                                url.short_name()) :
-          url.short_name();
+                                url_short_name) : url_short_name;
+    }
 
-    case IDS_SEARCH_ENGINES_EDITOR_KEYWORD_COLUMN:
-      return url.keyword();
+    case IDS_SEARCH_ENGINES_EDITOR_KEYWORD_COLUMN: {
+      const std::wstring& keyword = url.keyword();
+      // Keyword should be domain name. Force it to have LTR directionality.
+      if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
+        std::wstring localized_keyword = keyword;
+        l10n_util::WrapStringWithLTRFormatting(&localized_keyword);
+        return localized_keyword;
+      }
+      return keyword;
       break;
+    }
 
     default:
       NOTREACHED();
diff --git a/chrome/browser/views/options/cookies_view.cc b/chrome/browser/views/options/cookies_view.cc
index c019a9e..f1af2d8 100644
--- a/chrome/browser/views/options/cookies_view.cc
+++ b/chrome/browser/views/options/cookies_view.cc
@@ -149,14 +149,23 @@ std::wstring CookiesTableModel::GetText(int row, int column_id) {
         // Domain cookies start with a trailing dot, but we will show this
         // in the cookie details, show it without the dot in the list.
         std::string& domain = shown_cookies_.at(row)->first;
+        std::wstring wide_domain;
         if (!domain.empty() && domain[0] == '.')
-          return UTF8ToWide(domain.substr(1));
-        return UTF8ToWide(domain);
+          wide_domain = UTF8ToWide(domain.substr(1));
+        else
+          wide_domain = UTF8ToWide(domain);
+        // Force domain to be LTR
+        if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+          l10n_util::WrapStringWithLTRFormatting(&wide_domain);
+        return wide_domain;
       }
       break;
-    case IDS_COOKIES_NAME_COLUMN_HEADER:
-      return UTF8ToWide(shown_cookies_.at(row)->second.Name());
+    case IDS_COOKIES_NAME_COLUMN_HEADER: {
+      std::wstring name = UTF8ToWide(shown_cookies_.at(row)->second.Name());
+      l10n_util::AdjustStringForLocaleDirection(name, &name);
+      return name;
       break;
+    }
   }
   NOTREACHED();
   return L"";
diff --git a/chrome/browser/views/options/general_page_view.cc b/chrome/browser/views/options/general_page_view.cc
index e819b1e..e8f19f5 100644
--- a/chrome/browser/views/options/general_page_view.cc
+++ b/chrome/browser/views/options/general_page_view.cc
@@ -300,6 +300,8 @@ int CustomHomePagesTableModel::RowCount() {
 std::wstring CustomHomePagesTableModel::GetText(int row, int column_id) {
   DCHECK(column_id == 0);
   DCHECK(row >= 0 && row < RowCount());
+  // No need to force URL to have LTR directionality because the custom home
+  // pages control is created using LTR directionality.
   return UTF8ToWide(entries_[row].url.spec());
 }
 
diff --git a/chrome/browser/views/password_manager_view.cc b/chrome/browser/views/password_manager_view.cc
index ab8c701..15e5771 100644
--- a/chrome/browser/views/password_manager_view.cc
+++ b/chrome/browser/views/password_manager_view.cc
@@ -73,10 +73,21 @@ int PasswordManagerTableModel::RowCount() {
 std::wstring PasswordManagerTableModel::GetText(int row,
                                                 int col_id) {
   switch (col_id) {
-    case IDS_PASSWORD_MANAGER_VIEW_SITE_COLUMN:  // Site.
-      return saved_signons_[row]->display_url.display_url();
-    case IDS_PASSWORD_MANAGER_VIEW_USERNAME_COLUMN:  // Username.
-      return GetPasswordFormAt(row)->username_value;
+    case IDS_PASSWORD_MANAGER_VIEW_SITE_COLUMN: {  // Site.
+      const std::wstring& url = saved_signons_[row]->display_url.display_url();
+      // Force URL to have LTR directionality.
+      if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
+        std::wstring localized_url = url;
+        l10n_util::WrapStringWithLTRFormatting(&localized_url);
+        return localized_url;
+      }
+      return url;
+    }
+    case IDS_PASSWORD_MANAGER_VIEW_USERNAME_COLUMN: {  // Username.
+      std::wstring username = GetPasswordFormAt(row)->username_value;
+      l10n_util::AdjustStringForLocaleDirection(username, &username);
+      return username;
+    }
     default:
       NOTREACHED() << "Invalid column.";
       return std::wstring();
diff --git a/chrome/browser/views/shelf_item_dialog.cc b/chrome/browser/views/shelf_item_dialog.cc
index 87b1b49..215e73b 100644
--- a/chrome/browser/views/shelf_item_dialog.cc
+++ b/chrome/browser/views/shelf_item_dialog.cc
@@ -131,12 +131,26 @@ class PossibleURLModel : public views::TableModel {
       return std::wstring();
     }
 
-    if (col_id == IDS_ASI_PAGE_COLUMN)
-      return GetTitle(row);
+    if (col_id == IDS_ASI_PAGE_COLUMN) {
+      const std::wstring& title = GetTitle(row);
+      // TODO(xji): Consider adding a special case if the title text is a URL,
+      // since those should always have LTR directionality. Please refer to
+      // http://crbug.com/6726 for more information.
+      std::wstring localized_title;
+      if (l10n_util::AdjustStringForLocaleDirection(title, &localized_title))
+        return localized_title;
+      return title;
+    }
 
     // TODO(brettw): this should probably pass the GURL up so the URL elider
     // can be used at a higher level when we know the width.
-    return results_[row].display_url.display_url();
+    const std::wstring& url = results_[row].display_url.display_url();
+    if (l10n_util::GetTextDirection() == l10n_util::LEFT_TO_RIGHT)
+      return url;
+    // Force URL to be LTR.
+    std::wstring localized_url = url;
+    l10n_util::WrapStringWithLTRFormatting(&localized_url);
+    return localized_url;
   }
 
   virtual SkBitmap GetIcon(int row) {