diff options
author | jeremy@chromium.org <jeremy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-18 06:51:15 +0000 |
---|---|---|
committer | jeremy@chromium.org <jeremy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-05-18 06:51:15 +0000 |
commit | c7e136fc8a564f1a30ef6214e4f2b5d598cfbe94 (patch) | |
tree | 147504d5e33aabc8c91d3be94ddec28f13eaead5 /chrome | |
parent | bd209fefecce23870a49ad6e848625f6eee7e66c (diff) | |
download | chromium_src-c7e136fc8a564f1a30ef6214e4f2b5d598cfbe94.zip chromium_src-c7e136fc8a564f1a30ef6214e4f2b5d598cfbe94.tar.gz chromium_src-c7e136fc8a564f1a30ef6214e4f2b5d598cfbe94.tar.bz2 |
Hide translate infobar on programmatic navigation.
In Google news, you can change the UI language through a popup menu [which does so by submitting a form programmatically]. The translation infobar wasn't being refreshed in this situation because the page navigation being performed is programmatic.
Infobar hiding on navigation, goes through TabContents::ExpireInfoBars() which has an early return if the navigation just performed wasn't initiated by the user. After this, the code loops through the list of infobars and calls ShouldExpire() on each one to determine if it can be closed.
This CL removes the early return and moves the code to determine dismissal to the ShouldExpire() method of the infobar delegates, thus allowing the translate infobar to indicate that it can be dismissed on page navigation.
BUG=70261
TEST=See bug.
Review URL: http://codereview.chromium.org/6883299
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85731 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/omnibox_search_hint.cc | 2 | ||||
-rw-r--r-- | chrome/browser/tab_contents/infobar_delegate.cc | 11 | ||||
-rw-r--r-- | chrome/browser/tab_contents/infobar_delegate.h | 8 | ||||
-rw-r--r-- | chrome/browser/translate/translate_infobar_delegate.cc | 10 | ||||
-rw-r--r-- | chrome/browser/translate/translate_infobar_delegate.h | 4 | ||||
-rw-r--r-- | chrome/browser/translate/translate_manager_browsertest.cc | 7 | ||||
-rw-r--r-- | chrome/browser/ui/browser_init.cc | 2 | ||||
-rw-r--r-- | chrome/browser/ui/cocoa/keystone_infobar.mm | 2 | ||||
-rw-r--r-- | chrome/browser/ui/tab_contents/tab_contents_wrapper.cc | 6 |
9 files changed, 33 insertions, 19 deletions
diff --git a/chrome/browser/omnibox_search_hint.cc b/chrome/browser/omnibox_search_hint.cc index 375db47..6121dc60 100644 --- a/chrome/browser/omnibox_search_hint.cc +++ b/chrome/browser/omnibox_search_hint.cc @@ -100,7 +100,7 @@ HintInfoBar::~HintInfoBar() { bool HintInfoBar::ShouldExpire( const NavigationController::LoadCommittedDetails& details) const { - return should_expire_; + return details.is_user_initiated_main_frame_load() && should_expire_; } void HintInfoBar::InfoBarDismissed() { diff --git a/chrome/browser/tab_contents/infobar_delegate.cc b/chrome/browser/tab_contents/infobar_delegate.cc index d0b04b7..d17bc11 100644 --- a/chrome/browser/tab_contents/infobar_delegate.cc +++ b/chrome/browser/tab_contents/infobar_delegate.cc @@ -21,6 +21,17 @@ bool InfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const { bool InfoBarDelegate::ShouldExpire( const NavigationController::LoadCommittedDetails& details) const { + // Only hide InfoBars when the user has done something that makes the main + // frame load. We don't want various automatic or subframe navigations making + // it disappear. + if (!details.is_user_initiated_main_frame_load()) + return false; + + return ShouldExpireInternal(details); +} + +bool InfoBarDelegate::ShouldExpireInternal( + const NavigationController::LoadCommittedDetails& details) const { return (contents_unique_id_ != details.entry->unique_id()) || (PageTransition::StripQualifier(details.entry->transition_type()) == PageTransition::RELOAD); diff --git a/chrome/browser/tab_contents/infobar_delegate.h b/chrome/browser/tab_contents/infobar_delegate.h index ea56a1e..dd12472 100644 --- a/chrome/browser/tab_contents/infobar_delegate.h +++ b/chrome/browser/tab_contents/infobar_delegate.h @@ -52,8 +52,8 @@ class InfoBarDelegate { virtual bool EqualsDelegate(InfoBarDelegate* delegate) const; // Returns true if the InfoBar should be closed automatically after the page - // is navigated. The default behavior is to return true if the page is - // navigated somewhere else or reloaded. + // is navigated. The default behavior is to return true if the user initiated + // navigation somewhere else or reloaded. virtual bool ShouldExpire( const NavigationController::LoadCommittedDetails& details) const; @@ -91,6 +91,10 @@ class InfoBarDelegate { // be expired from |contents_|. void StoreActiveEntryUniqueID(TabContents* contents); + // Returns true if the navigation is to a new URL or a reload occured. + bool ShouldExpireInternal( + const NavigationController::LoadCommittedDetails& details) const; + private: // The unique id of the active NavigationEntry of the TabContents that we were // opened for. Used to help expire on navigations. diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc index 3c39bce..76e6a4b 100644 --- a/chrome/browser/translate/translate_infobar_delegate.cc +++ b/chrome/browser/translate/translate_infobar_delegate.cc @@ -353,6 +353,16 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate( } } +bool TranslateInfoBarDelegate::ShouldExpire( + const NavigationController::LoadCommittedDetails& details) const { + // Note: we allow closing this infobar even if the main frame navigation + // was programmatic and not initiated by the user - crbug.com/70261 . + if (!details.is_user_initiated_main_frame_load() && !details.is_main_frame) + return false; + + return InfoBarDelegate::ShouldExpireInternal(details); +} + void TranslateInfoBarDelegate::InfoBarDismissed() { if (type_ != BEFORE_TRANSLATE) return; diff --git a/chrome/browser/translate/translate_infobar_delegate.h b/chrome/browser/translate/translate_infobar_delegate.h index 03c196e..4ba31e6 100644 --- a/chrome/browser/translate/translate_infobar_delegate.h +++ b/chrome/browser/translate/translate_infobar_delegate.h @@ -9,7 +9,7 @@ #include <string> #include <vector> -#include "chrome/browser/tab_contents/confirm_infobar_delegate.h" +#include "chrome/browser/tab_contents/infobar_delegate.h" #include "chrome/browser/translate/translate_prefs.h" #include "chrome/common/translate_errors.h" @@ -163,6 +163,8 @@ class TranslateInfoBarDelegate : public InfoBarDelegate { virtual void InfoBarDismissed() OVERRIDE; virtual gfx::Image* GetIcon() const OVERRIDE; virtual InfoBarDelegate::Type GetInfoBarType() const OVERRIDE; + virtual bool ShouldExpire( + const NavigationController::LoadCommittedDetails& details) const; virtual TranslateInfoBarDelegate* AsTranslateInfoBarDelegate() OVERRIDE; // Gets the host of the page being translated, or an empty string if no URL is diff --git a/chrome/browser/translate/translate_manager_browsertest.cc b/chrome/browser/translate/translate_manager_browsertest.cc index 797b2c6..e6da867 100644 --- a/chrome/browser/translate/translate_manager_browsertest.cc +++ b/chrome/browser/translate/translate_manager_browsertest.cc @@ -722,13 +722,6 @@ TEST_F(TranslateManagerTest, TranslateInPageNavigation) { infobar = GetTranslateInfoBar(); ASSERT_TRUE(infobar != NULL); - // Navigate in page, the same infobar should still be shown. - ClearRemovedInfoBars(); - SimulateNavigation(GURL("http://www.google.fr/#ref1"), "fr", - true); - EXPECT_FALSE(InfoBarRemoved()); - EXPECT_EQ(infobar, GetTranslateInfoBar()); - // Navigate out of page, a new infobar should show. // See note in TranslateCloseInfoBarInPageNavigation test on why it is // important to navigate to a page in a different language for this test. diff --git a/chrome/browser/ui/browser_init.cc b/chrome/browser/ui/browser_init.cc index ba68646..d825838 100644 --- a/chrome/browser/ui/browser_init.cc +++ b/chrome/browser/ui/browser_init.cc @@ -186,7 +186,7 @@ DefaultBrowserInfoBarDelegate::~DefaultBrowserInfoBarDelegate() { bool DefaultBrowserInfoBarDelegate::ShouldExpire( const NavigationController::LoadCommittedDetails& details) const { - return should_expire_; + return details.is_user_initiated_main_frame_load() && should_expire_; } gfx::Image* DefaultBrowserInfoBarDelegate::GetIcon() const { diff --git a/chrome/browser/ui/cocoa/keystone_infobar.mm b/chrome/browser/ui/cocoa/keystone_infobar.mm index d9cd41c..b51c622 100644 --- a/chrome/browser/ui/cocoa/keystone_infobar.mm +++ b/chrome/browser/ui/cocoa/keystone_infobar.mm @@ -84,7 +84,7 @@ KeystonePromotionInfoBarDelegate::~KeystonePromotionInfoBarDelegate() { bool KeystonePromotionInfoBarDelegate::ShouldExpire( const NavigationController::LoadCommittedDetails& details) const { - return can_expire_; + return details.is_user_initiated_main_frame_load() && can_expire_; } gfx::Image* KeystonePromotionInfoBarDelegate::GetIcon() const { diff --git a/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc b/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc index f81d1c5..ade9b15 100644 --- a/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc +++ b/chrome/browser/ui/tab_contents/tab_contents_wrapper.cc @@ -317,12 +317,6 @@ void TabContentsWrapper::Observe(NotificationType type, NavigationController::LoadCommittedDetails& committed_details = *(Details<NavigationController::LoadCommittedDetails>(details).ptr()); - // Only hide InfoBars when the user has done something that makes the main - // frame load. We don't want various automatic or subframe navigations - // making it disappear. - if (!committed_details.is_user_initiated_main_frame_load()) - return; - // NOTE: It is not safe to change the following code to count upwards or // use iterators, as the RemoveInfoBar() call synchronously modifies our // delegate list. |