diff options
author | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-05 18:27:01 +0000 |
---|---|---|
committer | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-05 18:27:01 +0000 |
commit | 34cec8d81015da03ed6c2b781800ac31488aa121 (patch) | |
tree | 58a21fcd43ca35a7a1db529e8f8562905d337990 | |
parent | c4890e9336ba3a059eb838c690d35c95e39fda90 (diff) | |
download | chromium_src-34cec8d81015da03ed6c2b781800ac31488aa121.zip chromium_src-34cec8d81015da03ed6c2b781800ac31488aa121.tar.gz chromium_src-34cec8d81015da03ed6c2b781800ac31488aa121.tar.bz2 |
Fix for bug 36463. Reloading a page by pressing enter in the
location bar would not bring back the translate infobar.
BUG=36463
TEST=Navigate to a page in a different language so it brings up
the translate infobar, close the infobar. Click on the
location bar and press enter. The page should be reloaded
and the translation infobar should be shown again.
Review URL: http://codereview.chromium.org/661301
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40754 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.cc | 6 | ||||
-rw-r--r-- | chrome/browser/translate/translate_manager.cc | 6 | ||||
-rw-r--r-- | chrome/browser/translate/translate_manager_unittest.cc | 66 |
3 files changed, 74 insertions, 4 deletions
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 254567c..891249a 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -1498,9 +1498,9 @@ void TabContents::DidNavigateAnyFramePostCommit( GetPasswordManager()->ProvisionallySavePassword(params.password_form); // Let the LanguageState clear its state. - language_state_.DidNavigate(details.entry->transition_type() == - PageTransition::RELOAD, - details.is_in_page); + bool reload = details.entry->transition_type() == PageTransition::RELOAD || + details.type == NavigationType::SAME_PAGE; + language_state_.DidNavigate(reload, details.is_in_page); } void TabContents::CloseConstrainedWindows() { diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc index 5470a8b..dd08b50 100644 --- a/chrome/browser/translate/translate_manager.cc +++ b/chrome/browser/translate/translate_manager.cc @@ -37,13 +37,17 @@ void TranslateManager::Observe(NotificationType type, case NotificationType::NAV_ENTRY_COMMITTED: { NavigationController* controller = Source<NavigationController>(source).ptr(); + NavigationController::LoadCommittedDetails* load_details = + Details<NavigationController::LoadCommittedDetails>(details).ptr(); NavigationEntry* entry = controller->GetActiveEntry(); if (!entry) { NOTREACHED(); return; } - if (entry->transition_type() != PageTransition::RELOAD) + if (entry->transition_type() != PageTransition::RELOAD && + load_details->type != NavigationType::SAME_PAGE) { return; + } // When doing a page reload, we don't get a TAB_LANGUAGE_DETERMINED // notification. So we need to explictly initiate the translation. // Note that we delay it as the TranslateManager gets this notification diff --git a/chrome/browser/translate/translate_manager_unittest.cc b/chrome/browser/translate/translate_manager_unittest.cc index 476faa7..95211cd 100644 --- a/chrome/browser/translate/translate_manager_unittest.cc +++ b/chrome/browser/translate/translate_manager_unittest.cc @@ -149,6 +149,34 @@ class TranslateManagerTest : public RenderViewHostTestHarness, DISALLOW_COPY_AND_ASSIGN(TranslateManagerTest); }; +// An observer that keeps track of whether a navigation entry was committed. +class NavEntryCommittedObserver : public NotificationObserver { + public: + explicit NavEntryCommittedObserver(TabContents* tab_contents) { + registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, + Source<NavigationController>(&tab_contents->controller())); + } + + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK(type == NotificationType::NAV_ENTRY_COMMITTED); + details_ = + *(Details<NavigationController::LoadCommittedDetails>(details).ptr()); + } + + const NavigationController::LoadCommittedDetails& + get_load_commited_details() const { + return details_; + } + + private: + NavigationController::LoadCommittedDetails details_; + NotificationRegistrar registrar_; + + DISALLOW_COPY_AND_ASSIGN(NavEntryCommittedObserver); +}; + TEST_F(TranslateManagerTest, NormalTranslate) { // Simulate navigating to a page. SimulateNavigation(GURL("http://www.google.fr"), 0, L"Le Google", "fr"); @@ -267,7 +295,45 @@ TEST_F(TranslateManagerTest, Reload) { EXPECT_TRUE(CloseTranslateInfoBar()); // Reload should bring back the infobar. + NavEntryCommittedObserver nav_observer(contents()); Reload(); + + // Ensures it is really handled a reload. + const NavigationController::LoadCommittedDetails& nav_details = + nav_observer.get_load_commited_details(); + EXPECT_TRUE(nav_details.entry != NULL); // There was a navigation. + EXPECT_EQ(NavigationType::EXISTING_PAGE, nav_details.type); + + // The TranslateManager class processes the navigation entry committed + // notification in a posted task; process that task. + MessageLoop::current()->RunAllPending(); + EXPECT_TRUE(GetTranslateInfoBar() != NULL); +} + +// Test that reloading the page by way of typing again the URL in the +// location bar brings back the infobar. +TEST_F(TranslateManagerTest, ReloadFromLocationBar) { + GURL url("http://www.google.fr"); + + // Simulate navigating to a page and getting its language. + SimulateNavigation(url, 0, L"Le Google", "fr"); + + // Close the infobar. + EXPECT_TRUE(CloseTranslateInfoBar()); + + // Create a pending navigation and simulate a page load. That should be the + // equivalent of typing the URL again in the location bar. + NavEntryCommittedObserver nav_observer(contents()); + contents()->controller().LoadURL(url, GURL(), PageTransition::TYPED); + rvh()->SendNavigate(0, url); + + // Test that we are really getting a same page navigation, the test would be + // useless if it was not the case. + const NavigationController::LoadCommittedDetails& nav_details = + nav_observer.get_load_commited_details(); + EXPECT_TRUE(nav_details.entry != NULL); // There was a navigation. + EXPECT_EQ(NavigationType::SAME_PAGE, nav_details.type); + // The TranslateManager class processes the navigation entry committed // notification in a posted task; process that task. MessageLoop::current()->RunAllPending(); |