This change list fix issue 5926 -- RTL: Text in [New Tab] should be truncated from RightToLeft instead of LeftToRight.

(http://crbug.com/5926)

Basically, the issue is that in RTL locales, the thumbnail title etc. text will always be truncated from left, even if they are pure English. For example, "mail.yahoo.com" whose title is "Yahoo! the best web based email!" will be truncated to "best web-based email!".
We should be able to truncate the title as "Yahoo! the be...".

The fix is to set the direction style of each title in the [New Tab] html page for title to be displayed and truncated correctly.

The fix consists 2 part: new_tab_ui.cc in backend and new_tab.html in frontend.

1. new_tab_ui.cc
For thumbnail title, recent bookmark title, and recently closed tab titles, originally, we set "title" and "url" into the data we pass to JS in new_tab.html. The fix added the title's "direction" to the data. The direction is by default  "ltr" and is set as "rtl" when there is character having strong directionality in the title.

2. new_tab.html
2.1 set text-alignment in thumbnail-title, bookmark container, and recently closed container to be "right" for RTL locales.
2.2 explicitly set title's directionality in the above 3 sections.


Test:
1. open Chrome in RTL locales.
2. open the following pages:
http://yahoo.com
http://gmail.com
http://mail.yahoo.com
http://wikipedia.com
http://msdn.microsoft.com/en-us/default.aspx
http://arabic.arabia.msn.com/default.aspx
http://he.wikipedia.org/
3. bookmark, close some of them.
4. open [New Tab]




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

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

2 files changed, 64 insertions, 19 deletions

diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index fec1e35..e8191a5 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/chrome/browser/dom_ui/new_tab_ui.cc
@@ -116,11 +116,11 @@ class PaintTimer : public RenderWidgetHost::PaintObserver {
   DISALLOW_COPY_AND_ASSIGN(PaintTimer);
 };
 
-// Adds "url" and "title" keys on incoming dictionary, setting title
-// as the url as a fallback on empty title.
-void SetURLAndTitle(DictionaryValue* dictionary,
-                    const std::wstring& title,
-                    const GURL& gurl) {
+// Adds "url", "title", and "direction" keys on incoming dictionary, setting
+// title as the url as a fallback on empty title.
+void SetURLTitleAndDirection(DictionaryValue* dictionary,
+                             const std::wstring& title,
+                             const GURL& gurl) {
   std::wstring wstring_url = UTF8ToWide(gurl.spec());
   dictionary->SetString(L"url", wstring_url);
 
@@ -131,20 +131,41 @@ void SetURLAndTitle(DictionaryValue* dictionary,
     title_to_set = wstring_url;
   }
 
+  // We set the "dir" attribute of the title, so that in RTL locales, a LTR
+  // title is rendered left-to-right and truncated from the right. For example,
+  // the title of http://msdn.microsoft.com/en-us/default.aspx is "MSDN:
+  // Microsoft developer network". In RTL locales, in the [New Tab] page, if
+  // the "dir" of this title is not specified, it takes Chrome UI's
+  // directionality. So the title will be truncated as "soft developer
+  // network". Setting the "dir" attribute as "ltr" renders the truncated title
+  // as "MSDN: Microsoft D...". As another example, the title of
+  // http://yahoo.com is "Yahoo!". In RTL locales, in the [New Tab] page, the
+  // title will be rendered as "!Yahoo" if its "dir" attribute is not set to
+  // "ltr".
+  //
   // Since the title can contain BiDi text, we need to mark the text as either
   // RTL or LTR, depending on the characters in the string. If we use the URL
   // as the title, we mark the title as LTR since URLs are always treated as
-  // left to right strings.
+  // left to right strings. Simply setting the title's "dir" attribute works
+  // fine for rendering and truncating the title. However, it does not work for
+  // entire title within a tooltip when the mouse is over the title link.. For
+  // example, without LRE-PDF pair, the title "Yahoo!" will be rendered as
+  // "!Yahoo" within the tooltip when the mouse is over the title link.
+  std::wstring direction = kDefaultHtmlTextDirection;
   if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
     if (using_url_as_the_title) {
       l10n_util::WrapStringWithLTRFormatting(&title_to_set);
     } else {
-      bool success =
-          l10n_util::AdjustStringForLocaleDirection(title, &title_to_set);
-      DCHECK(success ? (title != title_to_set) : (title == title_to_set));
+      if (l10n_util::StringContainsStrongRTLChars(title)) {
+        l10n_util::WrapStringWithRTLFormatting(&title_to_set);
+        direction = kRTLHtmlTextDirection;
+      } else {
+        l10n_util::WrapStringWithLTRFormatting(&title_to_set);
+      }
     }
   }
   dictionary->SetString(L"title", title_to_set);
+  dictionary->SetString(L"direction", direction);
 }
 
 }  // end anonymous namespace
@@ -314,7 +335,7 @@ void MostVisitedHandler::OnSegmentUsageAvailable(
   for (size_t i = 0; i < count; ++i) {
     const PageUsageData& page = *(*data)[i];
     DictionaryValue* page_value = new DictionaryValue;
-    SetURLAndTitle(page_value, page.GetTitle(), page.GetURL());
+    SetURLTitleAndDirection(page_value, page.GetTitle(), page.GetURL());
     pages_value.Append(page_value);
     most_visited_urls_.push_back(page.GetURL());
   }
@@ -500,7 +521,7 @@ void RecentlyBookmarkedHandler::SendBookmarksToPage() {
   for (size_t i = 0; i < recently_bookmarked.size(); ++i) {
     BookmarkNode* node = recently_bookmarked[i];
     DictionaryValue* entry_value = new DictionaryValue;
-    SetURLAndTitle(entry_value, node->GetTitle(), node->GetURL());
+    SetURLTitleAndDirection(entry_value, node->GetTitle(), node->GetURL());
     list_value.Append(entry_value);
   }
   dom_ui_host_->CallJavascriptFunction(L"recentlyBookmarked", list_value);
@@ -640,8 +661,8 @@ bool RecentlyClosedTabsHandler::TabToValue(
   if (current_navigation.url() == NewTabUIURL())
     return false;
 
-  SetURLAndTitle(dictionary, current_navigation.title(),
-                 current_navigation.url());
+  SetURLTitleAndDirection(dictionary, current_navigation.title(),
+                          current_navigation.url());
   dictionary->SetString(L"type", L"tab");
   return true;
 }
diff --git a/chrome/browser/resources/new_tab.html b/chrome/browser/resources/new_tab.html
index f158358..c6b4c0d 100644
--- a/chrome/browser/resources/new_tab.html
+++ b/chrome/browser/resources/new_tab.html
@@ -252,6 +252,7 @@ html[dir='rtl'] .thumbnail-title {
   background-position:right;
   padding-left:0px;
   padding-right:22px;
+  text-align:right;
 }
 .thumbnail {
   width:195px;
@@ -353,9 +354,15 @@ html[dir='rtl'] #managesearcheslink {
 #recentlyBookmarked {
   background-color:#e1ecfe;
 }
+html[dir='rtl'] #recentlyBookmarkedContainer {
+  text-align:right;
+}
 #recentlyClosedContainer {
   position:relative;
 }
+html[dir='rtl'] #recentlyClosedContainer {
+  text-align:right;
+}
 #searches input {
   border:1px solid #7f9db9;
   background-repeat: no-repeat;
@@ -514,11 +521,12 @@ function bind(fn, selfObj, var_args) {
 }
 
 /* Return the DOM element for a "most visited" entry.
-   |page| should be an object with "title" and "url" fields. */
+   |page| should be an object with "title", "url", and "direction" fields. */
 function makeMostVisitedDOM(page, number) {
   /* The HTML we want looks like this:
     <a class="most-visited-item" href="URL" title="gmail.com">
-      <div class="thumbnail-title" style="background-image:url(faviconurl);">gmail.com</div>
+      <div class="thumbnail-title" 
+           style="background-image:url(faviconurl);direction:ltr">gmail.com</div>
       <img class="thumbnail" style="background-image:url(thumbnailurl);" />
     </a>
   */
@@ -543,6 +551,13 @@ function makeMostVisitedDOM(page, number) {
   var div_title = DOM('div', {className:'thumbnail-title'});
   div_title.style.backgroundImage =
       'url("chrome-ui://favicon/' + page.url + '")';
+  /* Set the title's directionality independently of the overall page
+     directionality. We need to do this since a purely LTR title should always
+     have it's direction set as ltr. We only set the title direction to rtl if
+     it contains a strong RTL character. Please refer to http://crbug.com/5926
+     for more information.
+  */
+  div_title.style.direction = page.direction;
   if (page.title) {
     div_title.appendChild(document.createTextNode(page.title));
   } else {
@@ -557,7 +572,8 @@ function makeMostVisitedDOM(page, number) {
 }
 
 /* This function is called by the browser with the most visited pages list.
-   |pages| is a list of page objects, which have url and title attributes. */
+   |pages| is a list of page objects, which have url, title, and direction
+   attributes. */
 function renderMostVisitedPages(pages) {
   logEvent('renderMostVisitedPages called: ' + pages.length);
 
@@ -654,8 +670,8 @@ function renderSearchURLs(urls) {
 }
 
 /* This function is called by the browser when the list of recently bookmarked
-   URLs is available.  |entries| is a list of objects with title and url
-   attributes. */
+   URLs is available.  |entries| is a list of objects with title, url, and
+   direction attributes. */
 function renderRecentlyBookmarked(entries) {
   logEvent('renderRecentlyBookmarked called: ' + entries.length);
   var section   = document.getElementById('recentlyBookmarked');
@@ -677,6 +693,10 @@ function renderRecentlyBookmarked(entries) {
           }, false);
       link.style.backgroundImage =
           'url("chrome-ui://favicon/' + entry.url + '")';
+      /* Set the bookmark title's directionality independently of the page, see
+         comment about setting div_title.style.direction above for details.
+      */
+      link.style.direction = entry.direction;
       link.appendChild(document.createTextNode(entry.title));
       container.appendChild(link);
     }
@@ -759,7 +779,7 @@ function renderRecentlyClosedTabs(entries) {
  * Creates an item to go in the recent bookmarks or recently closed lists.
  *
  * @param {String} tagName Tagname for the DOM element to create.
- * @param {Object} data Object with title and url to popuplate the element.
+ * @param {Object} data Object with title, url, and direction to popuplate the element.
  *
  * @return {Node} The element containing the bookmark.
  */
@@ -768,6 +788,10 @@ function createRecentBookmark(tagName, data) {
   if (tagName == 'a')
     link.href = data.url;
   link.style.backgroundImage = 'url("chrome-ui://favicon/' + data.url + '")';
+  /* Set the title's directionality independently of the page, see comment
+     about setting div_title.style.direction above for details.
+  */
+  link.style.direction = data.direction;
   link.appendChild(document.createTextNode(data.title));
   return link;
 }