From 80ea37d8369d4c6dc5a3fc64271762f97092629b Mon Sep 17 00:00:00 2001 From: "jcivelli@chromium.org" Date: Tue, 29 Jun 2010 18:45:02 +0000 Subject: Translate infobar always/never translate buttons Adding extra buttons on the translate infobar to let user easily set the "never/always translate this language" property when they declined/accepted translation for a language more than 3 times. Original review: http://codereview.chromium.org/2825015/show BUG=NONE TEST=Visit a page in a foreign language, translate the page. Repeat 4 times. On the fourth time, the "before translate" infobar should show a button that says "always translate". Similarly when declining translations 4 times. Review URL: http://codereview.chromium.org/2835025 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51148 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/sync/glue/synchronized_preferences.h | 2 + .../translate/translate_infobar_delegate2.cc | 48 ++++++++++++++---- .../translate/translate_infobar_delegate2.h | 12 +++++ .../translate/translate_manager2_unittest.cc | 58 ++++++++++++++++++++++ chrome/browser/translate/translate_prefs.cc | 52 +++++++++++++++++++ chrome/browser/translate/translate_prefs.h | 25 ++++++++++ .../views/infobars/before_translate_infobar.cc | 37 ++++++++++++++ .../views/infobars/before_translate_infobar.h | 2 + .../browser/views/infobars/infobar_text_button.cc | 7 +++ .../browser/views/infobars/infobar_text_button.h | 2 + 10 files changed, 235 insertions(+), 10 deletions(-) (limited to 'chrome/browser') diff --git a/chrome/browser/sync/glue/synchronized_preferences.h b/chrome/browser/sync/glue/synchronized_preferences.h index 1da42a7..c7a78e6 100644 --- a/chrome/browser/sync/glue/synchronized_preferences.h +++ b/chrome/browser/sync/glue/synchronized_preferences.h @@ -93,6 +93,8 @@ static const wchar_t* kSynchronizedPreferences[] = { TranslatePrefs::kPrefTranslateLanguageBlacklist, TranslatePrefs::kPrefTranslateSiteBlacklist, TranslatePrefs::kPrefTranslateWhitelists, + TranslatePrefs::kPrefTranslateDeniedCount, + TranslatePrefs::kPrefTranslateAcceptedCount, // Desktop notification permissions. prefs::kDesktopNotificationAllowedOrigins, diff --git a/chrome/browser/translate/translate_infobar_delegate2.cc b/chrome/browser/translate/translate_infobar_delegate2.cc index f27601a..7733b5f 100644 --- a/chrome/browser/translate/translate_infobar_delegate2.cc +++ b/chrome/browser/translate/translate_infobar_delegate2.cc @@ -126,6 +126,10 @@ bool TranslateInfoBarDelegate2::IsError() { } void TranslateInfoBarDelegate2::Translate() { + const std::string& original_language_code = GetOriginalLanguageCode(); + prefs_.ResetTranslationDeniedCount(original_language_code); + prefs_.IncrementTranslationAcceptedCount(original_language_code); + Singleton::get()->TranslatePage( tab_contents_, GetLanguageCodeAt(original_language_index()), @@ -138,6 +142,10 @@ void TranslateInfoBarDelegate2::RevertTranslation() { } void TranslateInfoBarDelegate2::TranslationDeclined() { + const std::string& original_language_code = GetOriginalLanguageCode(); + prefs_.ResetTranslationAcceptedCount(original_language_code); + prefs_.IncrementTranslationDeniedCount(original_language_code); + // Remember that the user declined the translation so as to prevent showing a // translate infobar for that page again. (TranslateManager initiates // translations when getting a LANGUAGE_DETERMINED from the page, which @@ -169,14 +177,11 @@ InfoBarDelegate::Type TranslateInfoBarDelegate2::GetInfoBarType() { } bool TranslateInfoBarDelegate2::IsLanguageBlacklisted() { - const std::string& original_lang = - GetLanguageCodeAt(original_language_index()); - return prefs_.IsLanguageBlacklisted(original_lang); + return prefs_.IsLanguageBlacklisted(GetOriginalLanguageCode()); } void TranslateInfoBarDelegate2::ToggleLanguageBlacklist() { - const std::string& original_lang = - GetLanguageCodeAt(original_language_index()); + const std::string& original_lang = GetOriginalLanguageCode(); if (prefs_.IsLanguageBlacklisted(original_lang)) { prefs_.RemoveLanguageFromBlacklist(original_lang); } else { @@ -209,14 +214,29 @@ bool TranslateInfoBarDelegate2::ShouldAlwaysTranslate() { } void TranslateInfoBarDelegate2::ToggleAlwaysTranslate() { - std::string original_lang = GetOriginalLanguageCode(); - std::string target_lang = GetTargetLanguageCode(); + const std::string& original_lang = GetOriginalLanguageCode(); + const std::string& target_lang = GetTargetLanguageCode(); if (prefs_.IsLanguagePairWhitelisted(original_lang, target_lang)) prefs_.RemoveLanguagePairFromWhitelist(original_lang, target_lang); else prefs_.WhitelistLanguagePair(original_lang, target_lang); } +void TranslateInfoBarDelegate2::AlwaysTranslatePageLanguage() { + const std::string& original_lang = GetOriginalLanguageCode(); + const std::string& target_lang = GetTargetLanguageCode(); + DCHECK(!prefs_.IsLanguagePairWhitelisted(original_lang, target_lang)); + prefs_.WhitelistLanguagePair(original_lang, target_lang); + Translate(); +} + +void TranslateInfoBarDelegate2::NeverTranslatePageLanguage() { + std::string original_lang = GetOriginalLanguageCode(); + DCHECK(!prefs_.IsLanguageBlacklisted(original_lang)); + prefs_.BlacklistLanguage(original_lang); + tab_contents_->RemoveInfoBar(this); +} + string16 TranslateInfoBarDelegate2::GetMessageInfoBarText() { switch (type_) { case TRANSLATING: @@ -257,9 +277,17 @@ string16 TranslateInfoBarDelegate2::GetMessageInfoBarButtonText() { void TranslateInfoBarDelegate2::MessageInfoBarButtonPressed() { DCHECK(type_ == TRANSLATION_ERROR); Singleton::get()->TranslatePage( - tab_contents_, - GetLanguageCodeAt(original_language_index()), - GetLanguageCodeAt(target_language_index())); + tab_contents_, GetOriginalLanguageCode(), GetTargetLanguageCode()); +} + +bool TranslateInfoBarDelegate2::ShouldShowNeverTranslateButton() { + DCHECK(type_ == BEFORE_TRANSLATE); + return prefs_.GetTranslationDeniedCount(GetOriginalLanguageCode()) >= 3; +} + +bool TranslateInfoBarDelegate2::ShouldShowAlwaysTranslateButton() { + DCHECK(type_ == BEFORE_TRANSLATE); + return prefs_.GetTranslationAcceptedCount(GetOriginalLanguageCode()) >= 3; } void TranslateInfoBarDelegate2::UpdateBackgroundAnimation( diff --git a/chrome/browser/translate/translate_infobar_delegate2.h b/chrome/browser/translate/translate_infobar_delegate2.h index 130c3d5..14c683a 100644 --- a/chrome/browser/translate/translate_infobar_delegate2.h +++ b/chrome/browser/translate/translate_infobar_delegate2.h @@ -105,12 +105,24 @@ class TranslateInfoBarDelegate2 : public InfoBarDelegate { virtual bool ShouldAlwaysTranslate(); virtual void ToggleAlwaysTranslate(); + // Methods called by the extra-buttons that can appear on the "before + // translate" infobar (when the user has accepted/declined the translation + // several times). + virtual void AlwaysTranslatePageLanguage(); + virtual void NeverTranslatePageLanguage(); + // The following methods are called by the infobar that displays the status // while translating and also the one displaying the error message. string16 GetMessageInfoBarText(); string16 GetMessageInfoBarButtonText(); void MessageInfoBarButtonPressed(); + // Called by the before translate infobar to figure-out if it should show + // an extra button to let the user black-list/white-list that language (based + // on how many times the user accepted/declined translation). + bool ShouldShowNeverTranslateButton(); + bool ShouldShowAlwaysTranslateButton(); + // Sets this infobar background animation based on the previous infobar shown. // A fading background effect is used when transitioning from a normal state // to an error state (and vice-versa). diff --git a/chrome/browser/translate/translate_manager2_unittest.cc b/chrome/browser/translate/translate_manager2_unittest.cc index 850041ae..7ad9f85 100644 --- a/chrome/browser/translate/translate_manager2_unittest.cc +++ b/chrome/browser/translate/translate_manager2_unittest.cc @@ -925,3 +925,61 @@ TEST_F(TranslateManager2Test, ContextMenu) { EXPECT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_TRANSLATE)); EXPECT_FALSE(menu->IsCommandIdEnabled(IDC_CONTENT_CONTEXT_TRANSLATE)); } + +// Tests that an extra always/never translate button is shown on the "before +// translate" infobar when the translation is accepted/declined 3 times. +TEST_F(TranslateManager2Test, BeforeTranslateExtraButtons) { + TranslatePrefs translate_prefs(contents()->profile()->GetPrefs()); + translate_prefs.ResetTranslationAcceptedCount("fr"); + translate_prefs.ResetTranslationDeniedCount("fr"); + translate_prefs.ResetTranslationAcceptedCount("de"); + translate_prefs.ResetTranslationDeniedCount("de"); + + TranslateInfoBarDelegate2* infobar; + for (int i = 0; i < 4; ++i) { + SimulateNavigation(GURL("http://www.google.fr"), 1, "Le Google", "fr"); + infobar = GetTranslateInfoBar(); + ASSERT_TRUE(infobar != NULL); + EXPECT_EQ(TranslateInfoBarDelegate2::BEFORE_TRANSLATE, infobar->type()); + if (i < 3) { + EXPECT_FALSE(infobar->ShouldShowAlwaysTranslateButton()); + infobar->Translate(); + process()->sink().ClearMessages(); + } else { + EXPECT_TRUE(infobar->ShouldShowAlwaysTranslateButton()); + } + } + // Simulate the user pressing "Always translate French". + infobar->AlwaysTranslatePageLanguage(); + EXPECT_TRUE(translate_prefs.IsLanguagePairWhitelisted("fr", "en")); + // Simulate the translate script being retrieved (it only needs to be done + // once in the test as it is cached). + SimulateURLFetch(true); + // That should have triggered a page translate. + int page_id = 0; + std::string original_lang, target_lang; + EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + process()->sink().ClearMessages(); + + // Now test that declining the translation causes a "never translate" button + // to be shown. + for (int i = 0; i < 4; ++i) { + SimulateNavigation(GURL("http://www.google.de"), 1, "Das Google", "de"); + infobar = GetTranslateInfoBar(); + ASSERT_TRUE(infobar != NULL); + EXPECT_EQ(TranslateInfoBarDelegate2::BEFORE_TRANSLATE, infobar->type()); + if (i < 3) { + EXPECT_FALSE(infobar->ShouldShowNeverTranslateButton()); + infobar->TranslationDeclined(); + } else { + EXPECT_TRUE(infobar->ShouldShowNeverTranslateButton()); + } + } + // Simulate the user pressing "Never translate French". + infobar->NeverTranslatePageLanguage(); + EXPECT_TRUE(translate_prefs.IsLanguageBlacklisted("de")); + // No translation should have occured and the infobar should be gone. + EXPECT_FALSE(GetTranslateMessage(&page_id, &original_lang, &target_lang)); + process()->sink().ClearMessages(); + ASSERT_TRUE(GetTranslateInfoBar() == NULL); +} diff --git a/chrome/browser/translate/translate_prefs.cc b/chrome/browser/translate/translate_prefs.cc index c1036b9..3d72df0 100644 --- a/chrome/browser/translate/translate_prefs.cc +++ b/chrome/browser/translate/translate_prefs.cc @@ -5,6 +5,7 @@ #include "chrome/browser/translate/translate_prefs.h" #include "base/string_util.h" +#include "base/utf_string_conversions.h" #include "chrome/browser/pref_service.h" #include "chrome/browser/scoped_pref_update.h" @@ -14,6 +15,10 @@ const wchar_t TranslatePrefs::kPrefTranslateSiteBlacklist[] = L"translate_site_blacklist"; const wchar_t TranslatePrefs::kPrefTranslateWhitelists[] = L"translate_whitelists"; +const wchar_t TranslatePrefs::kPrefTranslateDeniedCount[] = + L"translate_denied_count"; +const wchar_t TranslatePrefs::kPrefTranslateAcceptedCount[] = + L"translate_accepted_count"; // TranslatePrefs: public: ----------------------------------------------------- @@ -92,6 +97,49 @@ void TranslatePrefs::RemoveLanguagePairFromWhitelist( prefs_->ScheduleSavePersistentPrefs(); } +int TranslatePrefs::GetTranslationDeniedCount(const std::string& language) { + DictionaryValue* dict = + prefs_->GetMutableDictionary(kPrefTranslateDeniedCount); + int count = 0; + return dict->GetInteger(UTF8ToWide(language), &count) ? count : 0; +} + +void TranslatePrefs::IncrementTranslationDeniedCount( + const std::string& language) { + DictionaryValue* dict = + prefs_->GetMutableDictionary(kPrefTranslateDeniedCount); + int count = 0; + dict->GetInteger(UTF8ToWide(language), &count); + dict->SetInteger(UTF8ToWide(language), count + 1); +} + +void TranslatePrefs::ResetTranslationDeniedCount(const std::string& language) { + prefs_->GetMutableDictionary(kPrefTranslateDeniedCount)-> + SetInteger(UTF8ToWide(language), 0); +} + +int TranslatePrefs::GetTranslationAcceptedCount(const std::string& language) { + DictionaryValue* dict = + prefs_->GetMutableDictionary(kPrefTranslateAcceptedCount); + int count = 0; + return dict->GetInteger(UTF8ToWide(language), &count) ? count : 0; +} + +void TranslatePrefs::IncrementTranslationAcceptedCount( + const std::string& language) { + DictionaryValue* dict = + prefs_->GetMutableDictionary(kPrefTranslateAcceptedCount); + int count = 0; + dict->GetInteger(UTF8ToWide(language), &count); + dict->SetInteger(UTF8ToWide(language), count + 1); +} + +void TranslatePrefs::ResetTranslationAcceptedCount( + const std::string& language) { + prefs_->GetMutableDictionary(kPrefTranslateAcceptedCount)-> + SetInteger(UTF8ToWide(language), 0); +} + // TranslatePrefs: public, static: --------------------------------------------- bool TranslatePrefs::CanTranslate(PrefService* user_prefs, @@ -117,6 +165,10 @@ void TranslatePrefs::RegisterUserPrefs(PrefService* user_prefs) { user_prefs->RegisterDictionaryPref(kPrefTranslateWhitelists); MigrateTranslateWhitelists(user_prefs); } + if (!user_prefs->FindPreference(kPrefTranslateDeniedCount)) + user_prefs->RegisterDictionaryPref(kPrefTranslateDeniedCount); + if (!user_prefs->FindPreference(kPrefTranslateAcceptedCount)) + user_prefs->RegisterDictionaryPref(kPrefTranslateAcceptedCount); } // TranslatePrefs: private, static: -------------------------------------------- diff --git a/chrome/browser/translate/translate_prefs.h b/chrome/browser/translate/translate_prefs.h index fa8dc6d..9fece54 100644 --- a/chrome/browser/translate/translate_prefs.h +++ b/chrome/browser/translate/translate_prefs.h @@ -10,6 +10,7 @@ #include "googleurl/src/gurl.h" +class DictionaryValue; class ListValue; class PrefService; @@ -18,6 +19,8 @@ class TranslatePrefs { static const wchar_t kPrefTranslateLanguageBlacklist[]; static const wchar_t kPrefTranslateSiteBlacklist[]; static const wchar_t kPrefTranslateWhitelists[]; + static const wchar_t kPrefTranslateDeniedCount[]; + static const wchar_t kPrefTranslateAcceptedCount[]; explicit TranslatePrefs(PrefService* user_prefs); @@ -36,6 +39,20 @@ class TranslatePrefs { void RemoveLanguagePairFromWhitelist(const std::string& original_language, const std::string& target_language); + // These methods are used to track how many times the user has denied the + // translation for a specific language. (So we can present a UI to black-list + // that language if the user keeps denying translations). + int GetTranslationDeniedCount(const std::string& language); + void IncrementTranslationDeniedCount(const std::string& language); + void ResetTranslationDeniedCount(const std::string& language); + + // These methods are used to track how many times the user has accepted the + // translation for a specific language. (So we can present a UI to white-list + // that langueg if the user keeps accepting translations). + int GetTranslationAcceptedCount(const std::string& language); + void IncrementTranslationAcceptedCount(const std::string& language); + void ResetTranslationAcceptedCount(const std::string& language); + static bool CanTranslate(PrefService* user_prefs, const std::string& original_language, const GURL& url); static bool ShouldAutoTranslate(PrefService* user_prefs, @@ -52,6 +69,14 @@ class TranslatePrefs { bool IsLanguageWhitelisted(const std::string& original_language, std::string* target_language); + // Retrieves the dictionary mapping the number of times translation has been + // denied for a language, creating it if necessary. + DictionaryValue* GetTranslationDeniedCountDictionary(); + + // Retrieves the dictionary mapping the number of times translation has been + // accepted for a language, creating it if necessary. + DictionaryValue* GetTranslationAcceptedCountDictionary(); + PrefService* prefs_; // Weak. }; diff --git a/chrome/browser/views/infobars/before_translate_infobar.cc b/chrome/browser/views/infobars/before_translate_infobar.cc index 7f65180..a06a586 100644 --- a/chrome/browser/views/infobars/before_translate_infobar.cc +++ b/chrome/browser/views/infobars/before_translate_infobar.cc @@ -18,6 +18,8 @@ BeforeTranslateInfoBar::BeforeTranslateInfoBar( TranslateInfoBarDelegate2* delegate) : TranslateInfoBarBase(delegate), + never_translate_button_(NULL), + always_translate_button_(NULL), languages_menu_model_(delegate, LanguagesMenuModel2::ORIGINAL), options_menu_model_(delegate) { size_t offset = 0; @@ -49,6 +51,22 @@ BeforeTranslateInfoBar::BeforeTranslateInfoBar( false, this); AddChildView(options_menu_button_); + if (delegate->ShouldShowNeverTranslateButton()) { + const string16& language = delegate->GetLanguageDisplayableNameAt( + delegate->original_language_index()); + never_translate_button_ = InfoBarTextButton::CreateWithMessageIDAndParam( + this, IDS_TRANSLATE_INFOBAR_NEVER_TRANSLATE, language); + AddChildView(never_translate_button_); + } + + if (delegate->ShouldShowAlwaysTranslateButton()) { + const string16& language = delegate->GetLanguageDisplayableNameAt( + delegate->original_language_index()); + always_translate_button_ = InfoBarTextButton::CreateWithMessageIDAndParam( + this, IDS_TRANSLATE_INFOBAR_ALWAYS_TRANSLATE, language); + AddChildView(always_translate_button_); + } + UpdateOriginalButtonText(); } @@ -89,6 +107,20 @@ void BeforeTranslateInfoBar::Layout() { deny_button_->SetBounds( accept_button_->bounds().right() + InfoBar::kButtonButtonSpacing, OffsetY(this, pref_size), pref_size.width(), pref_size.height()); + + if (never_translate_button_) { + pref_size = never_translate_button_->GetPreferredSize(); + never_translate_button_->SetBounds( + deny_button_->bounds().right() + InfoBar::kButtonButtonSpacing, + OffsetY(this, pref_size), pref_size.width(), pref_size.height()); + } + if (always_translate_button_) { + DCHECK(!never_translate_button_); + pref_size = always_translate_button_->GetPreferredSize(); + always_translate_button_->SetBounds( + deny_button_->bounds().right() + InfoBar::kButtonButtonSpacing, + OffsetY(this, pref_size), pref_size.width(), pref_size.height()); + } } void BeforeTranslateInfoBar::ButtonPressed(views::Button* sender, @@ -97,6 +129,11 @@ void BeforeTranslateInfoBar::ButtonPressed(views::Button* sender, GetDelegate()->Translate(); } else if (sender == deny_button_) { RemoveInfoBar(); + GetDelegate()->TranslationDeclined(); + } else if (sender == never_translate_button_) { + GetDelegate()->NeverTranslatePageLanguage(); + } else if (sender == always_translate_button_) { + GetDelegate()->AlwaysTranslatePageLanguage(); } else { TranslateInfoBarBase::ButtonPressed(sender, event); } diff --git a/chrome/browser/views/infobars/before_translate_infobar.h b/chrome/browser/views/infobars/before_translate_infobar.h index 610fa0e..a89a6f2 100644 --- a/chrome/browser/views/infobars/before_translate_infobar.h +++ b/chrome/browser/views/infobars/before_translate_infobar.h @@ -63,6 +63,8 @@ class BeforeTranslateInfoBar views::MenuButton* options_menu_button_; InfoBarTextButton* accept_button_; InfoBarTextButton* deny_button_; + InfoBarTextButton* never_translate_button_; + InfoBarTextButton* always_translate_button_; scoped_ptr languages_menu_; LanguagesMenuModel2 languages_menu_model_; diff --git a/chrome/browser/views/infobars/infobar_text_button.cc b/chrome/browser/views/infobars/infobar_text_button.cc index 35726d0..ebf5d22 100644 --- a/chrome/browser/views/infobars/infobar_text_button.cc +++ b/chrome/browser/views/infobars/infobar_text_button.cc @@ -22,6 +22,13 @@ InfoBarTextButton* InfoBarTextButton::CreateWithMessageID( l10n_util::GetStringUTF16(message_id)); } +// static +InfoBarTextButton* InfoBarTextButton::CreateWithMessageIDAndParam( + views::ButtonListener* listener, int message_id, const string16& param) { + return new InfoBarTextButton(listener, + l10n_util::GetStringFUTF16(message_id, param)); +} + InfoBarTextButton::~InfoBarTextButton() { } diff --git a/chrome/browser/views/infobars/infobar_text_button.h b/chrome/browser/views/infobars/infobar_text_button.h index 698d2a4..221b5de 100644 --- a/chrome/browser/views/infobars/infobar_text_button.h +++ b/chrome/browser/views/infobars/infobar_text_button.h @@ -19,6 +19,8 @@ class InfoBarTextButton : public views::TextButton { // |message_id|. static InfoBarTextButton* CreateWithMessageID(views::ButtonListener* listener, int message_id); + static InfoBarTextButton* CreateWithMessageIDAndParam( + views::ButtonListener* listener, int message_id, const string16& param); virtual ~InfoBarTextButton(); -- cgit v1.1