Delays find-in-page until IME composition is committed.

chrome/browser/ui/views/find_bar_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/find_bar_view.h"
 
 #include <algorithm>
 
 #include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 377 matching lines @@
       break;
     default:
       NOTREACHED() << L"Unknown button";
       break;
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // FindBarView, views::TextfieldController implementation:
 
-void FindBarView::ContentsChanged(views::Textfield* sender,
-                                  const string16& new_contents) {
+bool FindBarView::HandleKeyEvent(views::Textfield* sender,
+                                 const ui::KeyEvent& key_event) {
+  // If the dialog is not visible, there is no reason to process keyboard input.
+  if (!host()->IsVisible())
+    return false;
+
+  if (find_bar_host()->MaybeForwardKeyEventToWebpage(key_event))
+    return true;  // Handled, we are done!
+
+  if (key_event.key_code() == ui::VKEY_RETURN) {
+    // Pressing Return/Enter starts the search (unless text box is empty).
+    string16 find_string = find_text_->text();
+    if (!find_string.empty()) {
+      FindBarController* controller = find_bar_host()->GetFindBarController();
+      FindTabHelper* find_tab_helper =
+          FindTabHelper::FromWebContents(controller->web_contents());
+      // Search forwards for enter, backwards for shift-enter.
+      find_tab_helper->StartFinding(find_string,
+                                    !key_event.IsShiftDown(),
+                                    false);  // Not case sensitive.
+    }
+    return true;
+  }
+
+  return false;
+}
+
+void FindBarView::OnAfterUserAction(views::Textfield* sender) {
   FindBarController* controller = find_bar_host()->GetFindBarController();
   DCHECK(controller);
   content::WebContents* web_contents = controller->web_contents();
   // We must guard against a NULL web_contents, which can happen if the text
   // in the Find box is changed right after the tab is destroyed. Otherwise, it
   // can lead to crashes, as exposed by automation testing in issue 8048.
   if (!web_contents)
     return;
   FindTabHelper* find_tab_helper = FindTabHelper::FromWebContents(web_contents);
 
+  // The composition text wouldn't be what the user is really looking for.
+  // We delay the search until the user commits the composition text.
+  if (sender->IsIMEComposing() ||
+      sender->text() == last_searched_text_)

[Peter Kasting, 2013/05/30 22:00:14]

Why check for the same text?

Right now, if you copy the text in the find bar and paste it back in, it will
re-find (and possibly re-ding at you) which seems correct to me.

[Yuki, 2013/05/31 08:58:25]

1. The original code used overriding TextfieldController::ContentsChanged(),
which is called only when the content text has changed.  So checking for the
same text keeps almost the same behavior as before.

2. It's possible that the text doesn't change after some editing.  For example,
when a user canceled the IME composition, the text doesn't change.

With this check for the same text, this method does nothing when you paste the
same text back in.  So re-find or re-ding doesn't happen.

With your comment, I understood crbug.com/79002 and added OnAfterPaste() hook in
Patch Set 2 so I can handle it.

+    return;
+
+  const string16& search_text = sender->text();
+  last_searched_text_ = search_text;
+
   // 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.empty()) {
+  if (!search_text.empty()) {
     // The last two params here are forward (true) and case sensitive (false).
-    find_tab_helper->StartFinding(new_contents, true, false);
+    find_tab_helper->StartFinding(search_text, true, false);
   } else {
     find_tab_helper->StopFinding(FindBarController::kClearSelectionOnPage);
     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
     // deleted. We can't do this on ChromeOS yet because we get ContentsChanged
     // sent for a lot more things than just the user nulling out the search
     // terms. See http://crbug.com/45372.
     Profile* profile =
         Profile::FromBrowserContext(web_contents->GetBrowserContext());
     FindBarState* find_bar_state = FindBarStateFactory::GetForProfile(profile);
     find_bar_state->set_last_prepopulate_text(string16());
   }
 }
 
-bool FindBarView::HandleKeyEvent(views::Textfield* sender,
-                                 const ui::KeyEvent& key_event) {
-  // If the dialog is not visible, there is no reason to process keyboard input.
-  if (!host()->IsVisible())
-    return false;
-
-  if (find_bar_host()->MaybeForwardKeyEventToWebpage(key_event))
-    return true;  // Handled, we are done!
-
-  if (key_event.key_code() == ui::VKEY_RETURN) {
-    // Pressing Return/Enter starts the search (unless text box is empty).
-    string16 find_string = find_text_->text();
-    if (!find_string.empty()) {
-      FindBarController* controller = find_bar_host()->GetFindBarController();
-      FindTabHelper* find_tab_helper =
-          FindTabHelper::FromWebContents(controller->web_contents());
-      // Search forwards for enter, backwards for shift-enter.
-      find_tab_helper->StartFinding(find_string,
-                                    !key_event.IsShiftDown(),
-                                    false);  // Not case sensitive.
-    }
-    return true;
-  }
-
-  return false;
-}
-
 void FindBarView::OnAfterCutOrCopy() {
   Profile* profile = host()->browser_view()->browser()->profile();
   ui::SourceTag source_tag =
       content::BrowserContext::GetMarkerForOffTheRecordContext(profile);
   if (source_tag != ui::SourceTag()) {
     // Overwrite the clipboard with the correct SourceTag
     ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread();
     string16 text;
     clipboard->ReadText(ui::Clipboard::BUFFER_STANDARD, &text);
 
@@ skipping 52 matching lines @@
 
 void FindBarView::OnThemeChanged() {
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   if (GetThemeProvider()) {
     close_button_->SetBackground(
         GetThemeProvider()->GetColor(ThemeProperties::COLOR_TAB_TEXT),
         rb.GetImageSkiaNamed(IDR_CLOSE_1),
         rb.GetImageSkiaNamed(IDR_CLOSE_1_MASK));
   }
 }