diff options
| -rw-r--r-- | base/string_util.cc | 4 | ||||
| -rw-r--r-- | base/string_util.h | 1 | ||||
| -rw-r--r-- | chrome/browser/cocoa/find_bar_cocoa_controller.mm | 3 | ||||
| -rw-r--r-- | chrome/browser/find_bar_controller.cc | 104 | ||||
| -rw-r--r-- | chrome/browser/find_bar_controller.h | 9 | ||||
| -rw-r--r-- | chrome/browser/gtk/find_bar_gtk.cc | 68 | ||||
| -rw-r--r-- | chrome/browser/gtk/find_bar_gtk.h | 8 | ||||
| -rw-r--r-- | chrome/browser/views/find_bar_view.cc | 28 | ||||
| -rw-r--r-- | chrome/browser/views/find_bar_view.h | 3 |
9 files changed, 144 insertions, 84 deletions
diff --git a/base/string_util.cc b/base/string_util.cc index 3a71ba4..6d9131b 100644 --- a/base/string_util.cc +++ b/base/string_util.cc @@ -1067,6 +1067,10 @@ std::wstring IntToWString(int value) { return IntToStringT<std::wstring, int, unsigned int, true>:: IntToString(value); } +string16 IntToString16(int value) { + return IntToStringT<string16, int, unsigned int, true>:: + IntToString(value); +} std::string UintToString(unsigned int value) { return IntToStringT<std::string, unsigned int, unsigned int, false>:: IntToString(value); diff --git a/base/string_util.h b/base/string_util.h index 84cb6e5..7df1d92 100644 --- a/base/string_util.h +++ b/base/string_util.h @@ -405,6 +405,7 @@ void ReplaceSubstringsAfterOffset(std::string* str, // Specialized string-conversion functions. std::string IntToString(int value); std::wstring IntToWString(int value); +string16 IntToString16(int value); std::string UintToString(unsigned int value); std::wstring UintToWString(unsigned int value); std::string Int64ToString(int64 value); diff --git a/chrome/browser/cocoa/find_bar_cocoa_controller.mm b/chrome/browser/cocoa/find_bar_cocoa_controller.mm index 43123bf..cedc89a 100644 --- a/chrome/browser/cocoa/find_bar_cocoa_controller.mm +++ b/chrome/browser/cocoa/find_bar_cocoa_controller.mm @@ -139,9 +139,6 @@ // otherwise match counts won't work on Mac and Linux. NSString* searchString = [findText_ stringValue]; if ([searchString length] > 0) { - // TODO(rohitrao): Implement similar logic as in FindBarWin, to - // avoid flickering when searching. For now, only update the - // results label if both our numbers are non-negative. if (result.active_match_ordinal() >= 0 && result.number_of_matches() >= 0) { [resultsLabel_ setStringValue:base::SysWideToNSString( l10n_util::GetStringF(IDS_FIND_IN_PAGE_COUNT, diff --git a/chrome/browser/find_bar_controller.cc b/chrome/browser/find_bar_controller.cc index 087cb5a..2d1a457 100644 --- a/chrome/browser/find_bar_controller.cc +++ b/chrome/browser/find_bar_controller.cc @@ -11,7 +11,9 @@ #include "chrome/browser/tab_contents/tab_contents.h" FindBarController::FindBarController(FindBar* find_bar) - : find_bar_(find_bar), tab_contents_(NULL) { + : find_bar_(find_bar), + tab_contents_(NULL), + last_reported_matchcount_(0) { } FindBarController::~FindBarController() { @@ -68,45 +70,45 @@ void FindBarController::ChangeTabContents(TabContents* contents) { find_bar_->Hide(false); } - if (tab_contents_) { - NotificationService::current()->AddObserver( - this, NotificationType::FIND_RESULT_AVAILABLE, - Source<TabContents>(tab_contents_)); - NotificationService::current()->AddObserver( - this, NotificationType::NAV_ENTRY_COMMITTED, - Source<NavigationController>(&tab_contents_->controller())); - - // Find out what we should show in the find text box. Usually, this will be - // the last search in this tab, but if no search has been issued in this tab - // we use the last search string (from any tab). - string16 find_string = tab_contents_->find_text(); - if (find_string.empty()) - find_string = tab_contents_->find_prepopulate_text(); - - // Update the find bar with existing results and search text, regardless of - // whether or not the find bar is visible, so that if it's subsequently - // shown it is showing the right state for this tab. We update the find text - // _first_ since the FindBarView checks its emptiness to see if it should - // clear the result count display when there's nothing in the box. - find_bar_->SetFindText(find_string); - - if (tab_contents_->find_ui_active()) { - // A tab with a visible find bar just got selected and we need to show the - // find bar but without animation since it was already animated into its - // visible state. We also want to reset the window location so that - // we don't surprise the user by popping up to the left for no apparent - // reason. - gfx::Rect new_pos = find_bar_->GetDialogPosition(gfx::Rect()); - find_bar_->SetDialogPosition(new_pos, false); - - // Only modify focus and selection if Find is active, otherwise the Find - // Bar will interfere with user input. - find_bar_->SetFocusAndSelection(); - } - - find_bar_->UpdateUIForFindResult(tab_contents_->find_result(), - tab_contents_->find_text()); + if (!tab_contents_) + return; + + NotificationService::current()->AddObserver( + this, NotificationType::FIND_RESULT_AVAILABLE, + Source<TabContents>(tab_contents_)); + NotificationService::current()->AddObserver( + this, NotificationType::NAV_ENTRY_COMMITTED, + Source<NavigationController>(&tab_contents_->controller())); + + // Find out what we should show in the find text box. Usually, this will be + // the last search in this tab, but if no search has been issued in this tab + // we use the last search string (from any tab). + string16 find_string = tab_contents_->find_text(); + if (find_string.empty()) + find_string = tab_contents_->find_prepopulate_text(); + + // Update the find bar with existing results and search text, regardless of + // whether or not the find bar is visible, so that if it's subsequently + // shown it is showing the right state for this tab. We update the find text + // _first_ since the FindBarView checks its emptiness to see if it should + // clear the result count display when there's nothing in the box. + find_bar_->SetFindText(find_string); + + if (tab_contents_->find_ui_active()) { + // A tab with a visible find bar just got selected and we need to show the + // find bar but without animation since it was already animated into its + // visible state. We also want to reset the window location so that + // we don't surprise the user by popping up to the left for no apparent + // reason. + gfx::Rect new_pos = find_bar_->GetDialogPosition(gfx::Rect()); + find_bar_->SetDialogPosition(new_pos, false); + + // Only modify focus and selection if Find is active, otherwise the Find + // Bar will interfere with user input. + find_bar_->SetFocusAndSelection(); } + + UpdateFindBarForCurrentResult(); } //////////////////////////////////////////////////////////////////////////////// @@ -119,8 +121,7 @@ void FindBarController::Observe(NotificationType type, // Don't update for notifications from TabContentses other than the one we // are actively tracking. if (Source<TabContents>(source).ptr() == tab_contents_) { - find_bar_->UpdateUIForFindResult(tab_contents_->find_result(), - tab_contents_->find_text()); + UpdateFindBarForCurrentResult(); if (tab_contents_->find_result().final_update() && tab_contents_->find_result().number_of_matches() == 0) { find_bar_->AudibleAlert(); @@ -149,3 +150,24 @@ void FindBarController::Observe(NotificationType type, } } } + +void FindBarController::UpdateFindBarForCurrentResult() { + const FindNotificationDetails& find_result = tab_contents_->find_result(); + + // Avoid bug 894389: When a new search starts (and finds something) it reports + // an interim match count result of 1 before the scoping effort starts. This + // is to provide feedback as early as possible that we will find something. + // As you add letters to the search term, this creates a flashing effect when + // we briefly show "1 of 1" matches because there is a slight delay until + // the scoping effort starts updating the match count. We avoid this flash by + // ignoring interim results of 1 if we already have a positive number. + if (find_result.number_of_matches() > -1) { + if (last_reported_matchcount_ > 0 && + find_result.number_of_matches() == 1 && + !find_result.final_update()) + return; // Don't let interim result override match count. + last_reported_matchcount_ = find_result.number_of_matches(); + } + + find_bar_->UpdateUIForFindResult(find_result, tab_contents_->find_text()); +} diff --git a/chrome/browser/find_bar_controller.h b/chrome/browser/find_bar_controller.h index fc286ef..5d8edf8 100644 --- a/chrome/browser/find_bar_controller.h +++ b/chrome/browser/find_bar_controller.h @@ -40,11 +40,20 @@ class FindBarController : public NotificationObserver { FindBar* find_bar() const { return find_bar_.get(); } private: + // Sents an update to the find bar with the tab contents' current result. The + // tab_contents_ must be non-NULL before this call. Theis handles + // de-flickering in addition to just calling the update function. + void UpdateFindBarForCurrentResult(); + scoped_ptr<FindBar> find_bar_; // The TabContents we are currently associated with. Can be NULL. TabContents* tab_contents_; + // The last match count we reported to the user. This is used by + // UpdateFindBarForCurrentResult to avoid flickering. + int last_reported_matchcount_; + DISALLOW_COPY_AND_ASSIGN(FindBarController); }; diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc index 3c27bdd..23e735e 100644 --- a/chrome/browser/gtk/find_bar_gtk.cc +++ b/chrome/browser/gtk/find_bar_gtk.cc @@ -73,9 +73,9 @@ FindBarGtk::FindBarGtk(BrowserWindowGtk* browser) // Hook up signals after the widget has been added to the hierarchy so the // widget will be realized. - g_signal_connect(find_text_, "changed", + g_signal_connect(text_entry_, "changed", G_CALLBACK(OnChanged), this); - g_signal_connect(find_text_, "key-press-event", + g_signal_connect(text_entry_, "key-press-event", G_CALLBACK(OnKeyPressEvent), this); g_signal_connect(widget(), "size-allocate", G_CALLBACK(OnFixedSizeAllocate), this); @@ -144,20 +144,30 @@ void FindBarGtk::InitWidgets() { gtk_box_pack_end(GTK_BOX(hbox), find_previous_button_->widget(), FALSE, FALSE, 0); - find_text_ = gtk_entry_new(); + // Make a box for the edit and match count widgets. This is fixed size since + // we want the widgets inside to resize themselves rather than making the + // dialog bigger. + GtkWidget* content_hbox = gtk_hbox_new(false, 0); + gtk_widget_set_size_request(content_hbox, 300, -1); + + text_entry_ = gtk_entry_new(); + match_count_label_ = gtk_label_new(NULL); + // Force the text widget height so it lines up with the buttons regardless of // font size. - gtk_widget_set_size_request(find_text_, -1, 20); - gtk_entry_set_has_frame(GTK_ENTRY(find_text_), FALSE); + gtk_widget_set_size_request(text_entry_, -1, 20); + gtk_entry_set_has_frame(GTK_ENTRY(text_entry_), FALSE); + + gtk_box_pack_end(GTK_BOX(content_hbox), match_count_label_, FALSE, FALSE, 0); + gtk_box_pack_end(GTK_BOX(content_hbox), text_entry_, TRUE, TRUE, 0); // We fake anti-aliasing by having two borders. - GtkWidget* border_bin = gfx::CreateGtkBorderBin(find_text_, + GtkWidget* border_bin = gfx::CreateGtkBorderBin(content_hbox, &kTextBorderColor, 1, 1, 1, 0); GtkWidget* border_bin_aa = gfx::CreateGtkBorderBin(border_bin, &kTextBorderColorAA, 1, 1, 1, 0); - GtkWidget* centering_vbox = gtk_vbox_new(FALSE, 0); gtk_box_pack_start(GTK_BOX(centering_vbox), border_bin_aa, TRUE, FALSE, 0); gtk_box_pack_end(GTK_BOX(hbox), centering_vbox, FALSE, FALSE, 0); @@ -165,6 +175,7 @@ void FindBarGtk::InitWidgets() { // We show just the GtkFixed and |border_| (but not the dialog). gtk_widget_show(widget()); gtk_widget_show(border_); + gtk_widget_show(match_count_label_); } GtkWidget* FindBarGtk::slide_widget() { @@ -175,7 +186,7 @@ void FindBarGtk::Show() { slide_widget_->Open(); if (container_->window) gdk_window_raise(container_->window); - gtk_widget_grab_focus(find_text_); + gtk_widget_grab_focus(text_entry_); } void FindBarGtk::Hide(bool animate) { @@ -186,9 +197,9 @@ void FindBarGtk::Hide(bool animate) { } void FindBarGtk::SetFocusAndSelection() { - gtk_widget_grab_focus(find_text_); + gtk_widget_grab_focus(text_entry_); // Select all the text. - gtk_entry_select_region(GTK_ENTRY(find_text_), 0, -1); + gtk_entry_select_region(GTK_ENTRY(text_entry_), 0, -1); } void FindBarGtk::ClearResults(const FindNotificationDetails& results) { @@ -204,12 +215,41 @@ void FindBarGtk::MoveWindowIfNecessary(const gfx::Rect& selection_rect, } void FindBarGtk::SetFindText(const string16& find_text) { - std::string find_text_utf8 = UTF16ToUTF8(find_text); - gtk_entry_set_text(GTK_ENTRY(find_text_), find_text_utf8.c_str()); + std::string text_entry_utf8 = UTF16ToUTF8(find_text); + gtk_entry_set_text(GTK_ENTRY(text_entry_), text_entry_utf8.c_str()); } void FindBarGtk::UpdateUIForFindResult(const FindNotificationDetails& result, const string16& find_text) { + std::string text_entry_utf8 = UTF16ToUTF8(find_text); + bool have_valid_range = + result.number_of_matches() != -1 && result.active_match_ordinal() != -1; + + // If we don't have any results and something was passed in, then that means + // someone pressed F3 while the Find box was closed. In that case we need to + // repopulate the Find box with what was passed in. + std::string search_string(gtk_entry_get_text(GTK_ENTRY(text_entry_))); + if (search_string.empty() && !text_entry_utf8.empty()) { + gtk_entry_set_text(GTK_ENTRY(text_entry_), text_entry_utf8.c_str()); + gtk_entry_select_region(GTK_ENTRY(text_entry_), 0, -1); + } + + if (!search_string.empty() && have_valid_range) { + gtk_label_set_text(GTK_LABEL(match_count_label_), + l10n_util::GetStringFUTF8(IDS_FIND_IN_PAGE_COUNT, + IntToString16(result.active_match_ordinal()), + IntToString16(result.number_of_matches())).c_str()); + } else { + // If there was no text entered, we don't show anything in the result count + // area. + gtk_label_set_text(GTK_LABEL(match_count_label_), ""); + } + + // TODO(brettw) We should update the background color of the text field to + // reflect whether any text was found. See the Windows version for how. + + // TODO(brettw) enable or disable the find next/previous buttons depending + // on whether any matches were found. } void FindBarGtk::AudibleAlert() { @@ -240,7 +280,7 @@ void FindBarGtk::RestoreSavedFocus() { // contents. // This function sometimes gets called when we don't have focus. We should do // nothing in this case. - if (!GTK_WIDGET_HAS_FOCUS(find_text_)) + if (!GTK_WIDGET_HAS_FOCUS(text_entry_)) return; find_bar_controller_->tab_contents()->Focus(); @@ -261,7 +301,7 @@ void FindBarGtk::FindEntryTextInContents(bool forward_search) { if (!tab_contents) return; - std::string new_contents(gtk_entry_get_text(GTK_ENTRY(find_text_))); + std::string new_contents(gtk_entry_get_text(GTK_ENTRY(text_entry_))); if (new_contents.length() > 0) { tab_contents->StartFinding(UTF8ToUTF16(new_contents), forward_search); diff --git a/chrome/browser/gtk/find_bar_gtk.h b/chrome/browser/gtk/find_bar_gtk.h index 585cf48..571d763 100644 --- a/chrome/browser/gtk/find_bar_gtk.h +++ b/chrome/browser/gtk/find_bar_gtk.h @@ -107,15 +107,21 @@ class FindBarGtk : public FindBar, bool container_shaped_; // The widget where text is entered. - GtkWidget* find_text_; + GtkWidget* text_entry_; // The next and previous match buttons. scoped_ptr<CustomDrawButton> find_previous_button_; scoped_ptr<CustomDrawButton> find_next_button_; + // The GtkLabel listing how many results were found. + GtkWidget* match_count_label_; + // The X to close the find bar. scoped_ptr<CustomDrawButton> close_button_; + // The last matchcount number we reported to the user. + int last_reported_matchcount_; + // Pointer back to the owning controller. FindBarController* find_bar_controller_; diff --git a/chrome/browser/views/find_bar_view.cc b/chrome/browser/views/find_bar_view.cc index f027535..aa640a5 100644 --- a/chrome/browser/views/find_bar_view.cc +++ b/chrome/browser/views/find_bar_view.cc @@ -90,7 +90,6 @@ FindBarView::FindBarView(FindBarWin* container) find_next_button_(NULL), close_button_(NULL), animation_offset_(0), - last_reported_matchcount_(0), toolbar_blend_(true) { ResourceBundle& rb = ResourceBundle::GetSharedInstance(); @@ -184,31 +183,16 @@ void FindBarView::UpdateForResult(const FindNotificationDetails& result, bool have_valid_range = result.number_of_matches() != -1 && result.active_match_ordinal() != -1; - // Avoid bug 894389: When a new search starts (and finds something) it reports - // an interim match count result of 1 before the scoping effort starts. This - // is to provide feedback as early as possible that we will find something. - // As you add letters to the search term, this creates a flashing effect when - // we briefly show "1 of 1" matches because there is a slight delay until - // the scoping effort starts updating the match count. We avoid this flash by - // ignoring interim results of 1 if we already have a positive number. - if (result.number_of_matches() > -1) { - if (last_reported_matchcount_ > 0 && - result.number_of_matches() == 1 && - !result.final_update()) - return; // Don't let interim result override match count. - last_reported_matchcount_ = result.number_of_matches(); - } - // If we don't have any results and something was passed in, then that means // someone pressed F3 while the Find box was closed. In that case we need to // repopulate the Find box with what was passed in. - if (find_text_->GetText().empty() && !find_text.empty()) { + std::wstring search_string = find_text_->GetText(); + if (search_string.empty() && !find_text.empty()) { find_text_->SetText(find_text); find_text_->SelectAll(); } - std::wstring search_string = find_text_->GetText(); - if (search_string.length() > 0 && have_valid_range) { + if (!search_string.empty() && have_valid_range) { match_count_text_->SetText( l10n_util::GetStringF(IDS_FIND_IN_PAGE_COUNT, IntToWString(result.active_match_ordinal()), @@ -427,7 +411,7 @@ void FindBarView::ButtonPressed(views::Button* sender) { switch (sender->tag()) { case FIND_PREVIOUS_TAG: case FIND_NEXT_TAG: - if (find_text_->GetText().length() > 0) { + if (!find_text_->GetText().empty()) { container_->GetFindBarController()->tab_contents()->StartFinding( find_text_->GetText(), sender->tag() == FIND_NEXT_TAG); @@ -463,7 +447,7 @@ void FindBarView::ContentsChanged(views::TextField* sender, // When the user changes something in the text box we check the contents and // if the textbox contains something we set it as the new search string and // initiate search (even though old searches might be in progress). - if (new_contents.length() > 0) { + if (!new_contents.empty()) { controller->tab_contents()->StartFinding(new_contents, true); } else { // The textbox is empty so we reset. true = clear selection on page. @@ -488,7 +472,7 @@ bool FindBarView::HandleKeystroke(views::TextField* sender, UINT message, case VK_RETURN: { // Pressing Return/Enter starts the search (unless text box is empty). std::wstring find_string = find_text_->GetText(); - if (find_string.length() > 0) { + if (!find_string.empty()) { // Search forwards for enter, backwards for shift-enter. container_->GetFindBarController()->tab_contents()->StartFinding( find_string, diff --git a/chrome/browser/views/find_bar_view.h b/chrome/browser/views/find_bar_view.h index 9987118..b87ffe1 100644 --- a/chrome/browser/views/find_bar_view.h +++ b/chrome/browser/views/find_bar_view.h @@ -109,9 +109,6 @@ class FindBarView : public views::View, views::ImageButton* find_next_button_; views::ImageButton* close_button_; - // The last matchcount number we reported to the user. - int last_reported_matchcount_; - // Whether or not we're attempting to blend with the toolbar (this is // false if the bookmarks bar is visible). bool toolbar_blend_; |