One of my recent CL added triggering form autofill when clicking an already focused text-field.

It was triggering form autofill or password autofill indiscriminately, causing a regression: double clicking a login text-field would trigger password autofill preventing the expected behavior of selecting the text in the text-field.  Only form autofill is expected to be triggered in that case.

BUG=9544
TEST=Open a page with a login form for which you have saved a password. Double-click the login text-field, this should select all the text. Ensure the form autocomplete still works as expected.
Review URL: http://codereview.chromium.org/62143

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@13408 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 25 insertions, 11 deletions

diff --git a/webkit/glue/editor_client_impl.cc b/webkit/glue/editor_client_impl.cc
index e48ebf5..b6a2f37 100644
--- a/webkit/glue/editor_client_impl.cc
+++ b/webkit/glue/editor_client_impl.cc
@@ -634,7 +634,7 @@ void EditorClientImpl::handleKeyboardEvent(WebCore::KeyboardEvent* evt) {
   if (evt->keyCode() == WebCore::VKEY_DOWN ||
       evt->keyCode() == WebCore::VKEY_UP) {
     DCHECK(evt->target()->toNode());
-    ShowAutofillForNode(evt->target()->toNode());
+    ShowFormAutofillForNode(evt->target()->toNode());
   }
 
   if (handleEditingKeyboardEvent(evt))
@@ -663,18 +663,20 @@ void EditorClientImpl::textDidChangeInTextField(WebCore::Element* element) {
   DCHECK(element->hasLocalName(WebCore::HTMLNames::inputTag));
   // Note that we only show the autofill popup in this case if the caret is at
   // the end.  This matches FireFox and Safari but not IE.
-  Autofill(static_cast<WebCore::HTMLInputElement*>(element), false, true);
+  Autofill(static_cast<WebCore::HTMLInputElement*>(element),
+           false, false, true);
 }
 
-bool EditorClientImpl::ShowAutofillForNode(WebCore::Node* node) {
+bool EditorClientImpl::ShowFormAutofillForNode(WebCore::Node* node) {
   WebCore::HTMLInputElement* input_element =
       webkit_glue::NodeToHTMLInputElement(node);
   if (input_element)
-    return Autofill(input_element, true, false);
+    return Autofill(input_element, true, true, false);
   return false;
 }
 
 bool EditorClientImpl::Autofill(WebCore::HTMLInputElement* input_element,
+                                bool form_autofill_only,
                                 bool autofill_on_empty_value,
                                 bool requires_caret_at_end) {
   // Cancel any pending DoAutofill calls.
@@ -695,7 +697,7 @@ bool EditorClientImpl::Autofill(WebCore::HTMLInputElement* input_element,
     return false;
 
   if (!requires_caret_at_end) {
-    DoAutofill(input_element, autofill_on_empty_value,
+    DoAutofill(input_element, form_autofill_only, autofill_on_empty_value,
                false, backspace_pressed_);
   } else {
     // We post a task for doing the autofill as the caret position is not set
@@ -707,6 +709,7 @@ bool EditorClientImpl::Autofill(WebCore::HTMLInputElement* input_element,
         FROM_HERE,
         autofill_factory_.NewRunnableMethod(&EditorClientImpl::DoAutofill,
                                             input_element,
+                                            form_autofill_only,
                                             autofill_on_empty_value,
                                             true,
                                             backspace_pressed_));
@@ -715,6 +718,7 @@ bool EditorClientImpl::Autofill(WebCore::HTMLInputElement* input_element,
 }
 
 void EditorClientImpl::DoAutofill(WebCore::HTMLInputElement* input_element,
+                                  bool form_autofill_only,
                                   bool autofill_on_empty_value,
                                   bool requires_caret_at_end,
                                   bool backspace) {
@@ -732,11 +736,16 @@ void EditorClientImpl::DoAutofill(WebCore::HTMLInputElement* input_element,
   }
 
   // First let's see if there is a password listener for that element.
+  // We won't trigger form autofill in that case, as having both behavior on
+  // a node would be confusing.
   WebFrameImpl* webframe =
       WebFrameImpl::FromFrame(input_element->document()->frame());
   webkit_glue::PasswordAutocompleteListener* listener =
       webframe->GetPasswordListener(input_element);
   if (listener) {
+    if (form_autofill_only)
+      return;
+
     if (backspace)  // No autocomplete for password on backspace.
       return;
 
diff --git a/webkit/glue/editor_client_impl.h b/webkit/glue/editor_client_impl.h
index d617fdf..4104a4b 100644
--- a/webkit/glue/editor_client_impl.h
+++ b/webkit/glue/editor_client_impl.h
@@ -115,18 +115,21 @@ class EditorClientImpl : public WebCore::EditorClient {
   virtual std::wstring Describe(WebCore::EAffinity affinity);
   virtual std::wstring Describe(WebCore::CSSStyleDeclaration* style);
 
-  // Shows the autofill popup for |node| if it is an HTMLInputElement and it is
-  // empty.  This is called when you press the up or down arrow in a text field
-  // or when clicking an already focused text-field.
+  // Shows the form autofill popup for |node| if it is an HTMLInputElement and
+  // it is empty.  This is called when you press the up or down arrow in a
+  // text-field or when clicking an already focused text-field.
   // Returns true if the autofill popup has been scheduled to be shown, false
   // otherwise.
-  virtual bool ShowAutofillForNode(WebCore::Node* node);
+  virtual bool ShowFormAutofillForNode(WebCore::Node* node);
 
  private:
   void ModifySelection(WebCore::Frame* frame,
                        WebCore::KeyboardEvent* event);
 
-  // Popups an autofill menu for |input_element| is applicable.
+  // Triggers autofill for |input_element| if applicable.  This can be form
+  // autofill (via a popup-menu) or password autofill depending on
+  // |input_element|.  If |form_autofill_only| is true, password autofill is not
+  // triggered.
   // |autofill_on_empty_value| indicates whether the autofill should be shown
   // when the text-field is empty.
   // If |requires_caret_at_end| is true, the autofill popup is only shown if the
@@ -134,6 +137,7 @@ class EditorClientImpl : public WebCore::EditorClient {
   // Returns true if the autofill popup has been scheduled to be shown, false
   // otherwise.
   bool Autofill(WebCore::HTMLInputElement* input_element,
+                bool form_autofill_only,
                 bool autofill_on_empty_value,
                 bool requires_caret_at_end);
 
@@ -142,6 +146,7 @@ class EditorClientImpl : public WebCore::EditorClient {
   // reflecting the last text change yet and we need it to decide whether or not
   // to show the autofill popup.
   void DoAutofill(WebCore::HTMLInputElement* input_element,
+                  bool form_autofill_only,
                   bool autofill_on_empty_value,
                   bool requires_caret_at_end,
                   bool backspace);
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index 440830f..c98ac6f 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -459,7 +459,7 @@ void WebViewImpl::MouseDown(const WebMouseEvent& event) {
   if (clicked_node.get() && clicked_node == GetFocusedNode()) {
     // Focus has not changed, show the autocomplete popup.
     static_cast<EditorClientImpl*>(page_->editorClient())->
-        ShowAutofillForNode(clicked_node.get());
+        ShowFormAutofillForNode(clicked_node.get());
   }
 
   // Dispatch the contextmenu event regardless of if the click was swallowed.