From 70edb8606702fd26565cc46fd3075bfe75f68d97 Mon Sep 17 00:00:00 2001 From: "tony@chromium.org" Date: Wed, 18 Aug 2010 16:23:18 +0000 Subject: [PATCH] Convert GetDisplayStringInLTRDirectionality from wstring to string16. BUG=23581 Review URL: http://codereview.chromium.org/3108027 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56535 0039d316-1c4b-4281-b951-d872f2087c98 --- app/text_elider.cc | 14 ++++++++++---- app/text_elider_unittest.cc | 9 +++++---- base/i18n/rtl.cc | 10 ++++++---- base/i18n/rtl.h | 7 +++++-- base/i18n/rtl_unittest.cc | 9 ++++----- chrome/browser/custom_home_pages_table_model.cc | 6 +++--- chrome/browser/download/download_item_model.cc | 9 +++++---- chrome/browser/download/download_util.cc | 7 ++++--- chrome/browser/possible_url_model.cc | 6 ++---- chrome/browser/renderer_host/render_view_host.cc | 4 +++- .../search_engines/template_url_table_model.cc | 6 +++--- chrome/browser/tab_contents/tab_contents.cc | 10 +++++----- chrome/browser/task_manager.cc | 7 +++---- chrome/browser/task_manager_resource_providers.cc | 3 ++- chrome/browser/views/bookmark_bar_view.cc | 3 ++- chrome/browser/views/download_item_view.cc | 4 +++- .../browser/views/options/passwords_page_view.cc | 3 ++- chrome/browser/views/status_bubble_views.cc | 4 +++- views/controls/label.cc | 3 ++- 19 files changed, 72 insertions(+), 52 deletions(-) diff --git a/app/text_elider.cc b/app/text_elider.cc index 1ee1d1cfe48022..995ac959596819 100644 --- a/app/text_elider.cc +++ b/app/text_elider.cc @@ -38,6 +38,12 @@ std::wstring CutString(const std::wstring& text, text.substr(text.length() - half_length, half_length); } +// TODO(tony): Get rid of wstrings. +std::wstring GetDisplayStringInLTRDirectionality(const std::wstring& text) { + return UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(text))); +} + } // namespace namespace gfx { @@ -285,7 +291,7 @@ std::wstring ElideFilename(const FilePath& filename, int full_width = font.GetStringWidth(filename.ToWStringHack()); if (full_width <= available_pixel_width) { std::wstring elided_name = filename.ToWStringHack(); - return base::i18n::GetDisplayStringInLTRDirectionality(&elided_name); + return GetDisplayStringInLTRDirectionality(elided_name); } #if defined(OS_WIN) @@ -299,7 +305,7 @@ std::wstring ElideFilename(const FilePath& filename, if (rootname.empty() || extension.empty()) { std::wstring elided_name = ElideText(filename.ToWStringHack(), font, available_pixel_width, false); - return base::i18n::GetDisplayStringInLTRDirectionality(&elided_name); + return GetDisplayStringInLTRDirectionality(elided_name); } int ext_width = font.GetStringWidth(extension); @@ -308,14 +314,14 @@ std::wstring ElideFilename(const FilePath& filename, // We may have trimmed the path. if (root_width + ext_width <= available_pixel_width) { std::wstring elided_name = rootname + extension; - return base::i18n::GetDisplayStringInLTRDirectionality(&elided_name); + return GetDisplayStringInLTRDirectionality(elided_name); } int available_root_width = available_pixel_width - ext_width; std::wstring elided_name = ElideText(rootname, font, available_root_width, false); elided_name += extension; - return base::i18n::GetDisplayStringInLTRDirectionality(&elided_name); + return GetDisplayStringInLTRDirectionality(elided_name); } // This function adds an ellipsis at the end of the text if the text diff --git a/app/text_elider_unittest.cc b/app/text_elider_unittest.cc index 627c556a219275..2dd76e290a0cf9 100644 --- a/app/text_elider_unittest.cc +++ b/app/text_elider_unittest.cc @@ -7,6 +7,7 @@ #include "base/i18n/rtl.h" #include "base/scoped_ptr.h" #include "base/string_util.h" +#include "base/utf_string_conversions.h" #include "gfx/font.h" #include "googleurl/src/gurl.h" #include "testing/gtest/include/gtest/gtest.h" @@ -178,11 +179,11 @@ TEST(TextEliderTest, TestFilenameEliding) { static const gfx::Font font; for (size_t i = 0; i < arraysize(testcases); ++i) { FilePath filepath(testcases[i].input); - std::wstring expected = testcases[i].output; - base::i18n::GetDisplayStringInLTRDirectionality(&expected); - EXPECT_EQ(expected, ElideFilename(filepath, + string16 expected = WideToUTF16(testcases[i].output); + expected = base::i18n::GetDisplayStringInLTRDirectionality(expected); + EXPECT_EQ(expected, WideToUTF16(ElideFilename(filepath, font, - font.GetStringWidth(testcases[i].output))); + font.GetStringWidth(testcases[i].output)))); } } diff --git a/base/i18n/rtl.cc b/base/i18n/rtl.cc index 6c3d39483834b7..f5381a221e811a 100644 --- a/base/i18n/rtl.cc +++ b/base/i18n/rtl.cc @@ -269,10 +269,12 @@ void WrapPathWithLTRFormatting(const FilePath& path, rtl_safe_path->push_back(kPopDirectionalFormatting); } -std::wstring GetDisplayStringInLTRDirectionality(std::wstring* text) { - if (IsRTL()) - WrapStringWithLTRFormatting(text); - return *text; +string16 GetDisplayStringInLTRDirectionality(const string16& text) { + if (!IsRTL()) + return text; + string16 text_mutable(text); + WrapStringWithLTRFormatting(&text_mutable); + return text_mutable; } const string16 StripWrappingBidiControlCharacters(const string16& text) { diff --git a/base/i18n/rtl.h b/base/i18n/rtl.h index 19b142cdc132ad..2fe932c4bcf0c0 100644 --- a/base/i18n/rtl.h +++ b/base/i18n/rtl.h @@ -6,6 +6,7 @@ #define BASE_I18N_RTL_H_ #pragma once +#include "base/compiler_specific.h" #include "base/string16.h" #include "build/build_config.h" @@ -135,14 +136,16 @@ void WrapPathWithLTRFormatting(const FilePath& path, // string is wrapped with LRE (Left-To-Right Embedding) and PDF (Pop // Directional Formatting) marks and returned. In LTR locale, the string itself // is returned. -std::wstring GetDisplayStringInLTRDirectionality(std::wstring* text); +string16 GetDisplayStringInLTRDirectionality(const string16& text) + WARN_UNUSED_RESULT; // Strip the beginning (U+202A..U+202B, U+202D..U+202E) and/or ending (U+202C) // explicit bidi control characters from |text|, if there are any. Otherwise, // return the text itself. Explicit bidi control characters display and have // semantic effect. They can be deleted so they might not always appear in a // pair. -const string16 StripWrappingBidiControlCharacters(const string16& text); +const string16 StripWrappingBidiControlCharacters(const string16& text) + WARN_UNUSED_RESULT; } // namespace i18n } // namespace base diff --git a/base/i18n/rtl_unittest.cc b/base/i18n/rtl_unittest.cc index 62de290adb08b5..061e1408a2605a 100644 --- a/base/i18n/rtl_unittest.cc +++ b/base/i18n/rtl_unittest.cc @@ -217,13 +217,12 @@ TEST_F(RTLTest, GetDisplayStringInLTRDirectionality) { { L"abc\x05d0\x05d1.jpg", L"\x202a"L"abc\x05d0\x05d1.jpg\x202c" }, }; for (unsigned int i = 0; i < arraysize(test_data); ++i) { - std::wstring input = test_data[i].raw_filename; - std::wstring expected = - base::i18n::GetDisplayStringInLTRDirectionality(&input); + string16 input = WideToUTF16(test_data[i].raw_filename); + string16 expected = base::i18n::GetDisplayStringInLTRDirectionality(input); if (base::i18n::IsRTL()) - EXPECT_EQ(test_data[i].display_string, expected); + EXPECT_EQ(expected, WideToUTF16(test_data[i].display_string)); else - EXPECT_EQ(input, expected); + EXPECT_EQ(expected, input); } } diff --git a/chrome/browser/custom_home_pages_table_model.cc b/chrome/browser/custom_home_pages_table_model.cc index e8f6b1bea4a36b..32cb7802ea1489 100644 --- a/chrome/browser/custom_home_pages_table_model.cc +++ b/chrome/browser/custom_home_pages_table_model.cc @@ -209,7 +209,7 @@ CustomHomePagesTableModel::Entry* std::wstring CustomHomePagesTableModel::FormattedURL(int row) const { std::wstring languages = UTF8ToWide(profile_->GetPrefs()->GetString(prefs::kAcceptLanguages)); - std::wstring url(net::FormatUrl(entries_[row].url, languages)); - base::i18n::GetDisplayStringInLTRDirectionality(&url); - return url; + string16 url = WideToUTF16(net::FormatUrl(entries_[row].url, languages)); + url = base::i18n::GetDisplayStringInLTRDirectionality(url); + return UTF16ToWide(url); } diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc index 8b58751a864890..c360273975b644 100644 --- a/chrome/browser/download/download_item_model.cc +++ b/chrome/browser/download/download_item_model.cc @@ -40,8 +40,9 @@ std::wstring DownloadItemModel::GetStatusText() { // as "MB 123/456" because it ends with an LTR run. In order to solve this, // we mark the total string as an LTR string if the UI layout is // right-to-left so that the string "456 MB" is treated as an LTR run. - std::wstring simple_total = FormatBytes(total, amount_units, true); - base::i18n::GetDisplayStringInLTRDirectionality(&simple_total); + string16 simple_total = WideToUTF16Hack(FormatBytes(total, amount_units, + true)); + simple_total = base::i18n::GetDisplayStringInLTRDirectionality(simple_total); TimeDelta remaining; string16 simple_time; @@ -74,8 +75,8 @@ std::wstring DownloadItemModel::GetStatusText() { true)); } else { status_text = l10n_util::GetStringFUTF16( - IDS_DOWNLOAD_STATUS_IN_PROGRESS, simple_size, - WideToUTF16Hack(simple_total), simple_time); + IDS_DOWNLOAD_STATUS_IN_PROGRESS, simple_size, simple_total, + simple_time); } } break; diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc index f61ac4c78f09a0..e76bf6ebf83ccd 100644 --- a/chrome/browser/download/download_util.cc +++ b/chrome/browser/download/download_util.cc @@ -548,9 +548,10 @@ DictionaryValue* CreateDownloadItemValue(DownloadItem* download, int id) { file_value->SetString("file_path", WideToUTF16Hack(download->full_path().ToWStringHack())); // Keep file names as LTR. - std::wstring file_name = download->GetFileName().ToWStringHack(); - base::i18n::GetDisplayStringInLTRDirectionality(&file_name); - file_value->SetString("file_name", WideToUTF16Hack(file_name)); + string16 file_name = WideToUTF16Hack( + download->GetFileName().ToWStringHack()); + file_name = base::i18n::GetDisplayStringInLTRDirectionality(file_name); + file_value->SetString("file_name", file_name); file_value->SetString("url", download->url().spec()); file_value->SetBoolean("otr", download->is_otr()); diff --git a/chrome/browser/possible_url_model.cc b/chrome/browser/possible_url_model.cc index c4a8561fa66131..44fccf112308b0 100644 --- a/chrome/browser/possible_url_model.cc +++ b/chrome/browser/possible_url_model.cc @@ -120,10 +120,8 @@ std::wstring PossibleURLModel::GetText(int row, int col_id) { // 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. - // Force URL to be LTR. - std::wstring url(UTF16ToWideHack(results_[row].display_url.display_url())); - base::i18n::GetDisplayStringInLTRDirectionality(&url); - return url; + string16 url = results_[row].display_url.display_url(); + return UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality(url)); } SkBitmap PossibleURLModel::GetIcon(int row) { diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index 51a0e299a08fc3..9c633d31c221c8 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -1334,7 +1334,9 @@ void RenderViewHost::OnMsgSetTooltipText( if (!tooltip_text.empty()) { if (text_direction_hint == WebKit::WebTextDirectionLeftToRight) { // Force the tooltip to have LTR directionality. - base::i18n::GetDisplayStringInLTRDirectionality(&wrapped_tooltip_text); + wrapped_tooltip_text = UTF16ToWide( + base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(wrapped_tooltip_text))); } else if (text_direction_hint == WebKit::WebTextDirectionRightToLeft && !base::i18n::IsRTL()) { // Force the tooltip to have RTL directionality. diff --git a/chrome/browser/search_engines/template_url_table_model.cc b/chrome/browser/search_engines/template_url_table_model.cc index 1192db07138cb0..dede39956b74fa 100644 --- a/chrome/browser/search_engines/template_url_table_model.cc +++ b/chrome/browser/search_engines/template_url_table_model.cc @@ -191,9 +191,9 @@ std::wstring TemplateURLTableModel::GetText(int row, int col_id) { case IDS_SEARCH_ENGINES_EDITOR_KEYWORD_COLUMN: { // Keyword should be domain name. Force it to have LTR directionality. - std::wstring keyword(url.keyword()); - base::i18n::GetDisplayStringInLTRDirectionality(&keyword); - return keyword; + string16 keyword = WideToUTF16(url.keyword()); + keyword = base::i18n::GetDisplayStringInLTRDirectionality(keyword); + return UTF16ToWide(keyword); } default: diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index dbcf1dded11747..9ee487be62b764 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -3162,14 +3162,14 @@ std::wstring TabContents::GetMessageBoxTitle(const GURL& frame_url, // TODO(brettw) it should be easier than this to do the correct language // handling without getting the accept language from the profile. - std::wstring base_address = gfx::ElideUrl(clean_url, gfx::Font(), 0, - UTF8ToWide(profile()->GetPrefs()->GetString(prefs::kAcceptLanguages))); + string16 base_address = WideToUTF16(gfx::ElideUrl(clean_url, gfx::Font(), 0, + UTF8ToWide(profile()->GetPrefs()->GetString(prefs::kAcceptLanguages)))); // Force URL to have LTR directionality. - base::i18n::GetDisplayStringInLTRDirectionality(&base_address); + base_address = base::i18n::GetDisplayStringInLTRDirectionality(base_address); - return l10n_util::GetStringF( + return UTF16ToWide(l10n_util::GetStringFUTF16( is_alert ? IDS_JAVASCRIPT_ALERT_TITLE : IDS_JAVASCRIPT_MESSAGEBOX_TITLE, - base_address); + base_address)); } gfx::NativeWindow TabContents::GetMessageBoxRootWindow() { diff --git a/chrome/browser/task_manager.cc b/chrome/browser/task_manager.cc index 0553ab0b842b35..fb236e3c2d4f64 100644 --- a/chrome/browser/task_manager.cc +++ b/chrome/browser/task_manager.cc @@ -126,11 +126,10 @@ string16 TaskManagerModel::GetResourceNetworkUsage(int index) const { return l10n_util::GetStringUTF16(IDS_TASK_MANAGER_NA_CELL_TEXT); if (net_usage == 0) return ASCIIToUTF16("0"); - std::wstring net_byte = - FormatSpeed(net_usage, GetByteDisplayUnits(net_usage), true); + string16 net_byte = WideToUTF16( + FormatSpeed(net_usage, GetByteDisplayUnits(net_usage), true)); // Force number string to have LTR directionality. - base::i18n::GetDisplayStringInLTRDirectionality(&net_byte); - return WideToUTF16Hack(net_byte); + return base::i18n::GetDisplayStringInLTRDirectionality(net_byte); } string16 TaskManagerModel::GetResourceCPUUsage(int index) const { diff --git a/chrome/browser/task_manager_resource_providers.cc b/chrome/browser/task_manager_resource_providers.cc index d3fa808d99f889..a9066c8c1fdc88 100644 --- a/chrome/browser/task_manager_resource_providers.cc +++ b/chrome/browser/task_manager_resource_providers.cc @@ -77,7 +77,8 @@ std::wstring TaskManagerTabContentsResource::GetTitle() const { if (tab_title.empty()) { tab_title = UTF8ToWide(tab_contents_->GetURL().spec()); // Force URL to be LTR. - base::i18n::GetDisplayStringInLTRDirectionality(&tab_title); + tab_title = UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(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 diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc index de33eb84274de3..0c205b0039bf51 100644 --- a/chrome/browser/views/bookmark_bar_view.cc +++ b/chrome/browser/views/bookmark_bar_view.cc @@ -158,7 +158,8 @@ static std::wstring CreateToolTipForURLAndTitle(const gfx::Point& screen_loc, // the Unicode BiDi algorithm puts certain characters on the left by // default. std::wstring elided_url(gfx::ElideUrl(url, tt_font, max_width, languages)); - base::i18n::GetDisplayStringInLTRDirectionality(&elided_url); + elided_url = UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(elided_url))); result.append(elided_url); } return result; diff --git a/chrome/browser/views/download_item_view.cc b/chrome/browser/views/download_item_view.cc index 7ceb5499599cbe..7dc9d5a87c708e 100644 --- a/chrome/browser/views/download_item_view.cc +++ b/chrome/browser/views/download_item_view.cc @@ -15,6 +15,7 @@ #include "base/i18n/rtl.h" #include "base/string_util.h" #include "base/sys_string_conversions.h" +#include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_theme_provider.h" #include "chrome/browser/download/download_item_model.h" @@ -285,7 +286,8 @@ DownloadItemView::DownloadItemView(DownloadItem* download, } else { ElideString(rootname, kFileNameMaxLength - extension.length(), &rootname); std::wstring filename = rootname + L"." + extension; - base::i18n::GetDisplayStringInLTRDirectionality(&filename); + filename = UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(filename))); dangerous_download_label_ = new views::Label( l10n_util::GetStringF(IDS_PROMPT_DANGEROUS_DOWNLOAD, filename)); } diff --git a/chrome/browser/views/options/passwords_page_view.cc b/chrome/browser/views/options/passwords_page_view.cc index a3396833c52be4..3b33c1b6929871 100644 --- a/chrome/browser/views/options/passwords_page_view.cc +++ b/chrome/browser/views/options/passwords_page_view.cc @@ -73,7 +73,8 @@ std::wstring PasswordsTableModel::GetText(int row, case IDS_PASSWORDS_PAGE_VIEW_SITE_COLUMN: { // Site. // Force URL to have LTR directionality. std::wstring url(saved_signons_[row]->display_url.display_url()); - base::i18n::GetDisplayStringInLTRDirectionality(&url); + url = UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(url))); return url; } case IDS_PASSWORDS_PAGE_VIEW_USERNAME_COLUMN: { // Username. diff --git a/chrome/browser/views/status_bubble_views.cc b/chrome/browser/views/status_bubble_views.cc index 1c40f54ca62dd9..e6f9075e40b7cd 100644 --- a/chrome/browser/views/status_bubble_views.cc +++ b/chrome/browser/views/status_bubble_views.cc @@ -12,6 +12,7 @@ #include "base/i18n/rtl.h" #include "base/message_loop.h" #include "base/string_util.h" +#include "base/utf_string_conversions.h" #include "chrome/browser/browser_theme_provider.h" #include "gfx/canvas_skia.h" #include "gfx/point.h" @@ -661,7 +662,8 @@ void StatusBubbleViews::SetURL(const GURL& url, const std::wstring& languages) { // An URL is always treated as a left-to-right string. On right-to-left UIs // we need to explicitly mark the URL as LTR to make sure it is displayed // correctly. - base::i18n::GetDisplayStringInLTRDirectionality(&url_text_); + url_text_ = UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(url_text_))); if (IsFrameVisible()) { view_->SetText(url_text_, true); diff --git a/views/controls/label.cc b/views/controls/label.cc index 7953d183a12f4c..c4f8b227030050 100644 --- a/views/controls/label.cc +++ b/views/controls/label.cc @@ -439,7 +439,8 @@ void Label::CalculateDrawStringParams(std::wstring* paint_text, // characters. We use the locale settings because an URL is always treated // as an LTR string, even if its containing view does not use an RTL UI // layout. - base::i18n::GetDisplayStringInLTRDirectionality(paint_text); + *paint_text = UTF16ToWide(base::i18n::GetDisplayStringInLTRDirectionality( + WideToUTF16(*paint_text))); } else if (elide_in_middle_) { *paint_text = gfx::ElideText(text_, font_, width(), true); } else {