From f4baafa9589f39be7c49b2db4a0cffde7681ff13 Mon Sep 17 00:00:00 2001 From: "jcivelli@chromium.org" Date: Fri, 16 Jul 2010 16:32:12 +0000 Subject: Adding a way for user to report errors in language detection Adding a menu to the translate infobar that let users report when the language of a page has been incorrectly detected. BUG=48739 TEST=Visit a page in a language that triggers the translate infobar. Select the menu to report an error. A new tab should be opened that references the page and language and let you report the error. Review URL: http://codereview.chromium.org/2911005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52683 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/chrome_dll_resource.h | 13 +++++----- chrome/app/generated_resources.grd | 3 +++ .../cocoa/translate/translate_infobar_unittest.mm | 13 ++++++---- chrome/browser/translate/options_menu_model.cc | 8 +++++++ .../translate/translate_infobar_delegate.cc | 10 +++++++- .../browser/translate/translate_infobar_delegate.h | 8 +++++++ chrome/browser/translate/translate_manager.cc | 28 ++++++++++++++++++++++ chrome/browser/translate/translate_manager.h | 5 ++++ 8 files changed, 77 insertions(+), 11 deletions(-) (limited to 'chrome') diff --git a/chrome/app/chrome_dll_resource.h b/chrome/app/chrome_dll_resource.h index 0029c03..80c1ec5 100644 --- a/chrome/app/chrome_dll_resource.h +++ b/chrome/app/chrome_dll_resource.h @@ -237,12 +237,13 @@ #define IDC_WRITING_DIRECTION_RTL 41123 // OSX only // Translate -#define IDC_TRANSLATE_OPTIONS_ALWAYS 42000 -#define IDC_TRANSLATE_OPTIONS_NEVER_TRANSLATE_LANG 42001 -#define IDC_TRANSLATE_OPTIONS_NEVER_TRANSLATE_SITE 42002 -#define IDC_TRANSLATE_OPTIONS_ABOUT 42003 -#define IDC_TRANSLATE_ORIGINAL_LANGUAGE_BASE 42100 -#define IDC_TRANSLATE_TARGET_LANGUAGE_BASE 42400 +#define IDC_TRANSLATE_OPTIONS_ALWAYS 42000 +#define IDC_TRANSLATE_OPTIONS_NEVER_TRANSLATE_LANG 42001 +#define IDC_TRANSLATE_OPTIONS_NEVER_TRANSLATE_SITE 42002 +#define IDC_TRANSLATE_REPORT_BAD_LANGUAGE_DETECTION 42003 +#define IDC_TRANSLATE_OPTIONS_ABOUT 42004 +#define IDC_TRANSLATE_ORIGINAL_LANGUAGE_BASE 42100 +#define IDC_TRANSLATE_TARGET_LANGUAGE_BASE 42400 // Next default values for new objects // diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 06d1d58..bed14b2 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -7087,6 +7087,9 @@ Keep your key file in a safe place. You will need it to create new versions of y Always translate $1French to $2German + + Not in $1French? Report this error + About Google Translate diff --git a/chrome/browser/cocoa/translate/translate_infobar_unittest.mm b/chrome/browser/cocoa/translate/translate_infobar_unittest.mm index 3b8ee65..4aea03b 100644 --- a/chrome/browser/cocoa/translate/translate_infobar_unittest.mm +++ b/chrome/browser/cocoa/translate/translate_infobar_unittest.mm @@ -152,7 +152,7 @@ TEST_F(TranslationInfoBarTest, OptionsMenuItemsHookedUp) { NSMenu* optionsMenu = [infobar_controller optionsMenu]; NSArray* optionsMenuItems = [optionsMenu itemArray]; - EXPECT_EQ([optionsMenuItems count], 5U); + EXPECT_EQ(7U, [optionsMenuItems count]); // First item is the options menu button's title, so there's no need to test // that the target on that is setup correctly. @@ -163,7 +163,9 @@ TEST_F(TranslationInfoBarTest, OptionsMenuItemsHookedUp) { NSMenuItem* alwaysTranslateLanguateItem = [optionsMenuItems objectAtIndex:1]; NSMenuItem* neverTranslateLanguateItem = [optionsMenuItems objectAtIndex:2]; NSMenuItem* neverTranslateSiteItem = [optionsMenuItems objectAtIndex:3]; - NSMenuItem* aboutTranslateItem = [optionsMenuItems objectAtIndex:4]; + // Separator at 4. + NSMenuItem* reportBadLanguageItem = [optionsMenuItems objectAtIndex:5]; + NSMenuItem* aboutTranslateItem = [optionsMenuItems objectAtIndex:6]; { EXPECT_CALL(*infobar_delegate, ToggleAlwaysTranslate()) @@ -184,8 +186,11 @@ TEST_F(TranslationInfoBarTest, OptionsMenuItemsHookedUp) { } { - // Can't mock this effectively, so just check that the tag is set correctly. - EXPECT_EQ([aboutTranslateItem tag], IDC_TRANSLATE_OPTIONS_ABOUT); + // Can't mock these effectively, so just check that the tag is set + // correctly. + EXPECT_EQ(IDC_TRANSLATE_REPORT_BAD_LANGUAGE_DETECTION, + [reportBadLanguageItem tag]); + EXPECT_EQ(IDC_TRANSLATE_OPTIONS_ABOUT, [aboutTranslateItem tag]); } } diff --git a/chrome/browser/translate/options_menu_model.cc b/chrome/browser/translate/options_menu_model.cc index 3453963..cd1ae2e 100644 --- a/chrome/browser/translate/options_menu_model.cc +++ b/chrome/browser/translate/options_menu_model.cc @@ -33,6 +33,10 @@ OptionsMenuModel::OptionsMenuModel( AddCheckItem(IDC_TRANSLATE_OPTIONS_NEVER_TRANSLATE_SITE, l10n_util::GetStringUTF16( IDS_TRANSLATE_INFOBAR_OPTIONS_NEVER_TRANSLATE_SITE)); + AddSeparator(); + AddItem(IDC_TRANSLATE_REPORT_BAD_LANGUAGE_DETECTION, + l10n_util::GetStringFUTF16(IDS_TRANSLATE_INFOBAR_OPTIONS_REPORT_ERROR, + original_language)); AddItemWithStringId(IDC_TRANSLATE_OPTIONS_ABOUT, IDS_TRANSLATE_INFOBAR_OPTIONS_ABOUT); } @@ -96,6 +100,10 @@ void OptionsMenuModel::ExecuteCommand(int command_id) { translate_infobar_delegate_->ToggleAlwaysTranslate(); break; + case IDC_TRANSLATE_REPORT_BAD_LANGUAGE_DETECTION: + translate_infobar_delegate_->ReportLanguageDetectionError(); + break; + case IDC_TRANSLATE_OPTIONS_ABOUT: { TabContents* tab_contents = translate_infobar_delegate_->tab_contents(); if (tab_contents) { diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc index b1fa79e..be616b4 100644 --- a/chrome/browser/translate/translate_infobar_delegate.cc +++ b/chrome/browser/translate/translate_infobar_delegate.cc @@ -57,6 +57,7 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate( background_animation_(kNone), tab_contents_(tab_contents), original_language_index_(-1), + initial_original_language_index_(-1), target_language_index_(-1), error_(error), infobar_view_(NULL), @@ -84,8 +85,10 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate( for (std::vector::const_iterator iter = languages_.begin(); iter != languages_.end(); ++iter) { std::string language_code = iter->first; - if (language_code == original_language) + if (language_code == original_language) { original_language_index_ = iter - languages_.begin(); + initial_original_language_index_ = original_language_index_; + } if (language_code == target_language) target_language_index_ = iter - languages_.begin(); } @@ -153,6 +156,11 @@ void TranslateInfoBarDelegate::RevertTranslation() { tab_contents_->RemoveInfoBar(this); } +void TranslateInfoBarDelegate::ReportLanguageDetectionError() { + Singleton::get()-> + ReportLanguageDetectionError(tab_contents_); +} + void TranslateInfoBarDelegate::TranslationDeclined() { const std::string& original_language_code = GetOriginalLanguageCode(); prefs_.ResetTranslationAcceptedCount(original_language_code); diff --git a/chrome/browser/translate/translate_infobar_delegate.h b/chrome/browser/translate/translate_infobar_delegate.h index 67c1283..c82cde7 100644 --- a/chrome/browser/translate/translate_infobar_delegate.h +++ b/chrome/browser/translate/translate_infobar_delegate.h @@ -89,6 +89,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate { virtual void Translate(); virtual void RevertTranslation(); + virtual void ReportLanguageDetectionError(); // Called when the user declines to translate a page, by either closing the // infobar or pressing the "Don't translate" button. @@ -180,6 +181,13 @@ class TranslateInfoBarDelegate : public InfoBarDelegate { // The index for language the page is originally in. int original_language_index_; + // The index for language the page is originally in that was originally + // reported (original_language_index_ changes if the user selects a new + // original language, but this one does not). This is necessary to report + // language detection errors with the right original language even if the user + // changed the original language. + int initial_original_language_index_; + // The index for language the page should be translated to. int target_language_index_; diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc index 073f0c0..d39578d 100644 --- a/chrome/browser/translate/translate_manager.cc +++ b/chrome/browser/translate/translate_manager.cc @@ -6,7 +6,10 @@ #include "app/resource_bundle.h" #include "base/compiler_specific.h" +#include "base/histogram.h" #include "base/string_util.h" +#include "chrome/browser/browser.h" +#include "chrome/browser/browser_list.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/pref_service.h" #include "chrome/browser/profile.h" @@ -27,6 +30,7 @@ #include "chrome/common/pref_names.h" #include "chrome/common/translate_errors.h" #include "grit/browser_resources.h" +#include "net/base/escape.h" #include "net/url_request/url_request_status.h" namespace { @@ -116,6 +120,8 @@ const char* const kTranslateScriptURL = "cb=cr.googleTranslate.onTranslateElementLoad"; const char* const kTranslateScriptHeader = "Google-Translate-Element-Mode: library"; +const char* const kReportLanguageDetectionErrorURL = + "http://translate.google.com/translate_error"; } // namespace @@ -422,6 +428,28 @@ void TranslateManager::RevertTranslation(TabContents* tab_contents) { tab_contents->language_state().original_language()); } +void TranslateManager::ReportLanguageDetectionError(TabContents* tab_contents) { + UMA_HISTOGRAM_COUNTS("Translate.ReportLanguageDetectionError", 1); + GURL page_url = tab_contents->controller().GetActiveEntry()->url(); + std::string report_error_url(kReportLanguageDetectionErrorURL); + report_error_url += "?client=cr&action=langidc&u="; + report_error_url += EscapeUrlEncodedData(page_url.spec()); + report_error_url += "&sl="; + report_error_url += tab_contents->language_state().original_language(); + report_error_url += "&hl="; + report_error_url += + GetLanguageCode(g_browser_process->GetApplicationLocale()); + // Open that URL in a new tab so that the user can tell us more. + Browser* browser = BrowserList::GetLastActive(); + if (!browser) { + NOTREACHED(); + return; + } + browser->AddTabWithURL(GURL(report_error_url), GURL(), + PageTransition::AUTO_BOOKMARK, -1, + TabStripModel::ADD_SELECTED, NULL, std::string()); +} + void TranslateManager::DoTranslatePage(TabContents* tab, const std::string& translate_script, const std::string& source_lang, diff --git a/chrome/browser/translate/translate_manager.h b/chrome/browser/translate/translate_manager.h index abc61e8..260f345 100644 --- a/chrome/browser/translate/translate_manager.h +++ b/chrome/browser/translate/translate_manager.h @@ -45,6 +45,11 @@ class TranslateManager : public NotificationObserver, // language. void RevertTranslation(TabContents* tab_contents); + // Reports to the Google translate server that a page language was incorrectly + // detected. This call is initiated by the user selecting the "report" menu + // under options in the translate infobar. + void ReportLanguageDetectionError(TabContents* tab_contents); + // Clears the translate script, so it will be fetched next time we translate. // Currently used by unit-tests. void ClearTranslateScript() { translate_script_.clear(); } -- cgit v1.1