diff options
author | pkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-16 00:01:17 +0000 |
---|---|---|
committer | pkotwicz@chromium.org <pkotwicz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-16 00:01:17 +0000 |
commit | 27e7a0550418ae72e0eea96ddca65d43f559103a (patch) | |
tree | 657431ef8613aae33a083d838c0d585476c414ee | |
parent | 5e9ca8a510914662f4795ac43fa0b35ef5e449d3 (diff) | |
download | chromium_src-27e7a0550418ae72e0eea96ddca65d43f559103a.zip chromium_src-27e7a0550418ae72e0eea96ddca65d43f559103a.tar.gz chromium_src-27e7a0550418ae72e0eea96ddca65d43f559103a.tar.bz2 |
The find bar can move off of its location at the right hand corner of the screen if it is obscuring text that it has found.
However, you can get the find bar to be empty and in the middle of the screen if you press the correct key combinations.
In the case of the 'Settings' page
1) Type a
2) Type backspace
This patch makes the find bar return to its home location at the right hand corner of the screen when it has no text.
The find bar should also go back to its home location if there are no matches. For instance:
Note, the find bar will stay in the center of the screen if 1) It has moved to the center of the screen to reveal selected text
2) More text is entered such that there is no 'found text'
eg type 'ae' on settings page.
BUG=116286
TEST=FindInPageControllerTest.FindMovesWhenObscuring
Review URL: https://chromiumcodereview.appspot.com/9564044
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127037 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/ui/find_bar/find_bar_host_browsertest.cc | 88 | ||||
-rw-r--r-- | chrome/browser/ui/find_bar/find_tab_helper.cc | 4 | ||||
-rw-r--r-- | chrome/browser/ui/gtk/find_bar_gtk.cc | 14 | ||||
-rw-r--r-- | chrome/browser/ui/views/find_bar_host.cc | 3 | ||||
-rw-r--r-- | chrome/browser/ui/views/find_bar_view.cc | 1 |
5 files changed, 70 insertions, 40 deletions
diff --git a/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc b/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc index 9dd9c9b..becea4a 100644 --- a/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc +++ b/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc @@ -129,19 +129,45 @@ class FindInPageControllerTest : public InProcessBrowserTest { void EnsureFindBoxOpen() { EnsureFindBoxOpenForBrowser(browser()); } -}; -// Platform independent FindInPage that takes |const wchar_t*| -// as an input. -int FindInPageWchar(TabContentsWrapper* tab, - const wchar_t* search_str, - bool forward, - bool case_sensitive, - int* ordinal) { - return ui_test_utils::FindInPage( - tab, WideToUTF16(std::wstring(search_str)), - forward, case_sensitive, ordinal); -} + // Platform independent FindInPage that takes |const wchar_t*| + // as an input. + int FindInPageWchar(TabContentsWrapper* tab, + const wchar_t* search_str, + bool forward, + bool case_sensitive, + int* ordinal) { + return ui_test_utils::FindInPage( + tab, WideToUTF16(std::wstring(search_str)), + forward, case_sensitive, ordinal); + } + + // Calls FindInPageWchar till the find box's x position != |start_x_position|. + // Return |start_x_position| if the find box has not moved after iterating + // through all matches of |search_str|. + int FindInPageTillBoxMoves(TabContentsWrapper* tab, + int start_x_position, + const wchar_t* search_str, + int expected_matches) { + // Search for |search_str| which the Find box is obscuring. + for (int index = 0; index < expected_matches; ++index) { + int ordinal = 0; + EXPECT_EQ(expected_matches, FindInPageWchar(tab, search_str, kFwd, + kIgnoreCase, &ordinal)); + + // Check the position. + bool fully_visible; + gfx::Point position; + EXPECT_TRUE(GetFindBarWindowInfo(&position, &fully_visible)); + EXPECT_TRUE(fully_visible); + + // If the Find box has moved then we are done. + if (position.x() != start_x_position) + return position.x(); + } + return start_x_position; + } +}; // This test loads a page with frames and starts FindInPage requests. IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageFrames) { @@ -668,30 +694,18 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindMovesWhenObscuring) { gfx::Point start_position; gfx::Point position; bool fully_visible = false; + int ordinal = 0; // Make sure it is open. EXPECT_TRUE(GetFindBarWindowInfo(&start_position, &fully_visible)); EXPECT_TRUE(fully_visible); - // Search for 'Chromium' which the Find box is obscuring. - int ordinal = 0; TabContentsWrapper* tab = browser()->GetSelectedTabContentsWrapper(); - int index = 0; - for (; index < kMoveIterations; ++index) { - EXPECT_EQ(kMoveIterations, FindInPageWchar(tab, L"Chromium", - kFwd, kIgnoreCase, &ordinal)); - - // Check the position. - EXPECT_TRUE(GetFindBarWindowInfo(&position, &fully_visible)); - EXPECT_TRUE(fully_visible); - - // If the Find box has moved then we are done. - if (position.x() != start_position.x()) - break; - } - // We should not have reached the end. - ASSERT_GT(kMoveIterations, index); + int moved_x_coord = FindInPageTillBoxMoves(tab, start_position.x(), + L"Chromium", kMoveIterations); + // The find box should have moved. + EXPECT_TRUE(moved_x_coord != start_position.x()); // Search for something guaranteed not to be obscured by the Find box. EXPECT_EQ(1, FindInPageWchar(tab, L"Done", @@ -702,6 +716,22 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindMovesWhenObscuring) { // Make sure Find box has moved back to its original location. EXPECT_EQ(position.x(), start_position.x()); + + // Move the find box again. + moved_x_coord = FindInPageTillBoxMoves(tab, start_position.x(), + L"Chromium", kMoveIterations); + EXPECT_TRUE(moved_x_coord != start_position.x()); + + // Search for an invalid string. + EXPECT_EQ(0, FindInPageWchar(tab, L"WeirdSearchString", + kFwd, kIgnoreCase, &ordinal)); + + // Check the position. + EXPECT_TRUE(GetFindBarWindowInfo(&position, &fully_visible)); + EXPECT_TRUE(fully_visible); + + // Make sure Find box has moved back to its original location. + EXPECT_EQ(position.x(), start_position.x()); } #if defined(OS_MACOSX) diff --git a/chrome/browser/ui/find_bar/find_tab_helper.cc b/chrome/browser/ui/find_bar/find_tab_helper.cc index 4847a194..c0b673a 100644 --- a/chrome/browser/ui/find_bar/find_tab_helper.cc +++ b/chrome/browser/ui/find_bar/find_tab_helper.cc @@ -140,7 +140,9 @@ void FindTabHelper::HandleFindReply(int request_id, active_match_ordinal = last_search_result_.active_match_ordinal(); gfx::Rect selection = selection_rect; - if (selection.IsEmpty()) + if (final_update && active_match_ordinal == 0) + selection = gfx::Rect(); + else if (selection_rect.IsEmpty()) selection = last_search_result_.selection_rect(); // Notify the UI, automation and any other observers that a find result was diff --git a/chrome/browser/ui/gtk/find_bar_gtk.cc b/chrome/browser/ui/gtk/find_bar_gtk.cc index 1ecfed6..a62846d 100644 --- a/chrome/browser/ui/gtk/find_bar_gtk.cc +++ b/chrome/browser/ui/gtk/find_bar_gtk.cc @@ -373,14 +373,12 @@ void FindBarGtk::SetFindText(const string16& find_text) { void FindBarGtk::UpdateUIForFindResult(const FindNotificationDetails& result, const string16& find_text) { - if (!result.selection_rect().IsEmpty()) { - selection_rect_ = result.selection_rect(); - int xposition = GetDialogPosition(result.selection_rect()).x(); - GtkAllocation allocation; - gtk_widget_get_allocation(widget(), &allocation); - if (xposition != allocation.x) - Reposition(); - } + selection_rect_ = result.selection_rect(); + int xposition = GetDialogPosition(result.selection_rect()).x(); + GtkAllocation allocation; + gtk_widget_get_allocation(widget(), &allocation); + if (xposition != allocation.x) + Reposition(); // Once we find a match we no longer want to keep track of what had // focus. EndFindSession will then set the focus to the page content. diff --git a/chrome/browser/ui/views/find_bar_host.cc b/chrome/browser/ui/views/find_bar_host.cc index 2705e24..c3e7018 100644 --- a/chrome/browser/ui/views/find_bar_host.cc +++ b/chrome/browser/ui/views/find_bar_host.cc @@ -142,8 +142,7 @@ void FindBarHost::UpdateUIForFindResult(const FindNotificationDetails& result, find_bar_view()->UpdateForResult(result, find_text); // We now need to check if the window is obscuring the search results. - if (!result.selection_rect().IsEmpty()) - MoveWindowIfNecessary(result.selection_rect(), false); + MoveWindowIfNecessary(result.selection_rect(), false); // Once we find a match we no longer want to keep track of what had // focus. EndFindSession will then set the focus to the page content. diff --git a/chrome/browser/ui/views/find_bar_view.cc b/chrome/browser/ui/views/find_bar_view.cc index b36d07e..5f3a12f 100644 --- a/chrome/browser/ui/views/find_bar_view.cc +++ b/chrome/browser/ui/views/find_bar_view.cc @@ -402,6 +402,7 @@ void FindBarView::ContentsChanged(views::Textfield* sender, } else { find_tab_helper->StopFinding(FindBarController::kClearSelection); UpdateForResult(find_tab_helper->find_result(), string16()); + find_bar_host()->MoveWindowIfNecessary(gfx::Rect(), false); // Clearing the text box should clear the prepopulate state so that when // we close and reopen the Find box it doesn't show the search we just |