Clicking a text field that is already focused triggers the autofill. However the code triggering this was just checking that the focused element before and after processing the event was the same.

We need to do a hit test to ensure the click is really on the text field, otherwise in cases where clicking somewhere in the page does not change the focus, we would bogusly bring up the autofill popup.

BUG=8627
TEST=Ensure autocomplete popup still works as expected: when  entering text, when using up/down arrows, when clicking selected text field. Also ensures the scenario from the bug does not trigger the popup.
Review URL: http://codereview.chromium.org/50038

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

4 files changed, 54 insertions, 27 deletions

diff --git a/webkit/glue/dom_operations.cc b/webkit/glue/dom_operations.cc
index 4773812..f0adc1a 100644
--- a/webkit/glue/dom_operations.cc
+++ b/webkit/glue/dom_operations.cc
@@ -312,14 +312,6 @@ static bool FillFormImpl(FormElements* fe, const FormData& data, bool submit) {
   return false;
 }
 
-// Helper function to cast a Node as an HTMLInputElement.
-static WebCore::HTMLInputElement* GetNodeAsInputElement(WebCore::Node* node) {
-  DCHECK(node->nodeType() == WebCore::Node::ELEMENT_NODE);
-  DCHECK(static_cast<WebCore::Element*>(node)->hasTagName(
-      WebCore::HTMLNames::inputTag));
-  return static_cast<WebCore::HTMLInputElement*>(node);
-}
-
 // Helper to search the given form element for the specified input elements
 // in |data|, and add results to |result|.
 static bool FindFormInputElements(WebCore::HTMLFormElement* fe,
@@ -346,7 +338,8 @@ static bool FindFormInputElements(WebCore::HTMLFormElement* fe,
     // matching elements it can get at them through the FormElement*.
     // Note: This assignment adds a reference to the InputElement.
     result->input_elements[data.elements[j]] =
-        GetNodeAsInputElement(temp_elements[0].get());
+        NodeToHTMLInputElement(temp_elements[0].get());
+    DCHECK(result->input_elements[data.elements[j]].get());
   }
   return true;
 }
@@ -874,4 +867,17 @@ int NumberOfActiveAnimations(WebView* view) {
   return controller->numberOfActiveAnimations();
 }
 
+WebCore::HTMLInputElement* ElementToHTMLInputElement(
+    WebCore::Element* element) {
+  if (!element->hasLocalName(WebCore::HTMLNames::inputTag))
+    return NULL;
+  return static_cast<WebCore::HTMLInputElement*>(element);
+}
+
+WebCore::HTMLInputElement* NodeToHTMLInputElement(WebCore::Node* node) {
+  if (node->nodeType() != WebCore::Node::ELEMENT_NODE)
+    return NULL;
+  return ElementToHTMLInputElement(static_cast<WebCore::Element*>(node));
+}
+
 } // webkit_glue
diff --git a/webkit/glue/dom_operations.h b/webkit/glue/dom_operations.h
index c6276de..a088ef7 100644
--- a/webkit/glue/dom_operations.h
+++ b/webkit/glue/dom_operations.h
@@ -12,6 +12,11 @@
 #include "googleurl/src/gurl.h"
 #include "webkit/glue/password_form_dom_manager.h"
 
+namespace WebCore {
+class Element;
+class Node;
+}
+
 struct FormData;
 class WebFrameImpl;
 class WebView;
@@ -146,6 +151,11 @@ bool ElementDoesAutoCompleteForElementWithId(WebView* view,
 // Returns the number of animations currently running.
 int NumberOfActiveAnimations(WebView* view);
 
+// Returns the passed element/node casted to an HTMLInputElement if it is one,
+// NULL if it is not an HTMLInputElement.
+WebCore::HTMLInputElement* ElementToHTMLInputElement(WebCore::Element* element);
+WebCore::HTMLInputElement* NodeToHTMLInputElement(WebCore::Node* node);
+
 }  // namespace webkit_glue
 
 #endif  // WEBKIT_GLUE_DOM_OPERATIONS_H__
diff --git a/webkit/glue/editor_client_impl.cc b/webkit/glue/editor_client_impl.cc
index 411c514..670bee0 100644
--- a/webkit/glue/editor_client_impl.cc
+++ b/webkit/glue/editor_client_impl.cc
@@ -28,6 +28,7 @@
 #include "base/string_util.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebKit.h"
 #include "webkit/glue/autofill_form.h"
+#include "webkit/glue/dom_operations.h"
 #include "webkit/glue/editor_client_impl.h"
 #include "webkit/glue/glue_util.h"
 #include "webkit/glue/webkit_glue.h"
@@ -664,14 +665,10 @@ void EditorClientImpl::textDidChangeInTextField(WebCore::Element* element) {
 }
 
 void EditorClientImpl::ShowAutofillForNode(WebCore::Node* node) {
-  if (node->nodeType() == WebCore::Node::ELEMENT_NODE) {
-    WebCore::Element* element = static_cast<WebCore::Element*>(node);
-    if (element->hasLocalName(WebCore::HTMLNames::inputTag)) {
-      WebCore::HTMLInputElement* input_element =
-          static_cast<WebCore::HTMLInputElement*>(element);
-      Autofill(input_element, true);
-    }
-  }
+  WebCore::HTMLInputElement* input_element =
+      webkit_glue::NodeToHTMLInputElement(node);
+  if (input_element)
+    Autofill(input_element, true);
 }
 
 void EditorClientImpl::Autofill(WebCore::HTMLInputElement* input_element,
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index 6a0b358..799f0d3 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -87,6 +87,7 @@ MSVC_POP_WARNING();
 #include "webkit/glue/chrome_client_impl.h"
 #include "webkit/glue/clipboard_conversion.h"
 #include "webkit/glue/context_menu_client_impl.h"
+#include "webkit/glue/dom_operations.h"
 #include "webkit/glue/dragclient_impl.h"
 #include "webkit/glue/editor_client_impl.h"
 #include "webkit/glue/event_conversion.h"
@@ -424,21 +425,34 @@ void WebViewImpl::MouseDown(const WebMouseEvent& event) {
     return;
 
   last_mouse_down_point_ = gfx::Point(event.x, event.y);
-  // We need to remember who has focus, as if the user left-clicks an already
-  // focused text-field, we may want to show the auto-fill popup.
-  RefPtr<Node> focused_node;
-  if (event.button == WebMouseEvent::BUTTON_LEFT)
-    focused_node = GetFocusedNode();
+
+  // If a text field that has focus is clicked again, we should display the
+  // autocomplete popup.
+  RefPtr<Node> clicked_node;
+  if (event.button == WebMouseEvent::BUTTON_LEFT) {
+    RefPtr<Node> focused_node = GetFocusedNode();
+    if (focused_node.get() &&
+        webkit_glue::NodeToHTMLInputElement(focused_node.get())) {
+      IntPoint point(event.x, event.y);
+      HitTestResult result(point);
+      result = page_->mainFrame()->eventHandler()->hitTestResultAtPoint(point,
+                                                                        false);
+      if (result.innerNonSharedNode() == focused_node) {
+        // Already focused text field was clicked, let's remember this.  If
+        // focus has not changed after the mouse event is processed, we'll
+        // trigger the autocomplete.
+        clicked_node = focused_node;
+      }
+    }
+  }
 
   main_frame()->frame()->eventHandler()->handleMousePressEvent(
       MakePlatformMouseEvent(main_frame()->frameview(), event));
 
-  if (focused_node.get() && focused_node == GetFocusedNode()) {
-    // Already focused node was clicked, ShowAutofillForNode will determine
-    // whether to show the autofill (typically, if the node is a text-field and
-    // is empty).
+  if (clicked_node.get() && clicked_node == GetFocusedNode()) {
+    // Focus has not changed, show the autocomplete popup.
     static_cast<EditorClientImpl*>(page_->editorClient())->
-        ShowAutofillForNode(focused_node.get());
+          ShowAutofillForNode(clicked_node.get());
   }
 }