diff options
author | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-05-14 15:53:13 +0000 |
---|---|---|
committer | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-05-14 15:53:13 +0000 |
commit | c07cfb808dbd642bb3385f974be296797bd34524 (patch) | |
tree | 2834f7923f749d5fdc1246888ca7caf5b0cf507f | |
parent | a68b966e94845efaa30dae71f4ca7c324eba57b9 (diff) | |
download | chromium_src-c07cfb808dbd642bb3385f974be296797bd34524.zip chromium_src-c07cfb808dbd642bb3385f974be296797bd34524.tar.gz chromium_src-c07cfb808dbd642bb3385f974be296797bd34524.tar.bz2 |
Add match count text to the GTK find bar. This also makes the bar wider.
The match count text doesn't currently show up realiably because the data in
the find reply is bad. I believe this is bug 11761.
The clicker checking code has been moved to the cross-platform
FindBarController so it doesn't have to be duplicated for each platform.
This also add IntToString16. I didnt add all the variants now. The *Wstring
versions should all eventually be changed to string16.
http://crbug.com/11750
Review URL: http://codereview.chromium.org/114023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16058 0039d316-1c4b-4281-b951-d872f2087c98
-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_; |