summaryrefslogtreecommitdiffstats
path: root/chrome/browser/views
diff options
context:
space:
mode:
authorxji@chromium.org <xji@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-01-23 23:13:05 +0000
committerxji@chromium.org <xji@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-01-23 23:13:05 +0000
commit2a5af8a6e448b72044353b691ab7766e989db152 (patch)
tree3cb7ec6be2f05d7154005e7da87ca08042f2e026 /chrome/browser/views
parent1926867a79a6bcf9817b4ddc9d30d3f348e2d2d5 (diff)
downloadchromium_src-2a5af8a6e448b72044353b691ab7766e989db152.zip
chromium_src-2a5af8a6e448b72044353b691ab7766e989db152.tar.gz
chromium_src-2a5af8a6e448b72044353b691ab7766e989db152.tar.bz2
===================================================
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
Diffstat (limited to 'chrome/browser/views')
-rw-r--r--chrome/browser/views/hung_renderer_view.cc4
-rw-r--r--chrome/browser/views/keyword_editor_view.cc24
-rw-r--r--chrome/browser/views/options/cookies_view.cc17
-rw-r--r--chrome/browser/views/options/general_page_view.cc2
-rw-r--r--chrome/browser/views/password_manager_view.cc19
-rw-r--r--chrome/browser/views/shelf_item_dialog.cc20
6 files changed, 70 insertions, 16 deletions
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) {