AutoFill: A few fixes.

* Match both the name and the label when sending FormData values for the renderer to fill.
* Use a vector of bools to indicate which WebFormControlElements are extracted instead of matching on element name, which is not guaranteed to be unique.

BUG=38325
TEST=FormManagerTest.InferredLabelsWithSameName

Review URL: http://codereview.chromium.org/1558029

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

2 files changed, 111 insertions, 8 deletions

diff --git a/chrome/renderer/form_manager.cc b/chrome/renderer/form_manager.cc
index 78fd488..87cfcea 100644
--- a/chrome/renderer/form_manager.cc
+++ b/chrome/renderer/form_manager.cc
@@ -114,6 +114,11 @@ bool FormManager::WebFormElementToFormData(const WebFormElement& element,
 
   WebVector<WebFormControlElement> control_elements;
   element.getFormControlElements(control_elements);
+
+  // A vector of bools that indicate whether each field in the form meets the
+  // requirements and thus will be in the resulting |form|.
+  std::vector<bool> fields_extracted(control_elements.size(), false);
+
   for (size_t i = 0; i < control_elements.size(); ++i) {
     const WebFormControlElement& control_element = control_elements[i];
 
@@ -136,6 +141,7 @@ bool FormManager::WebFormElementToFormData(const WebFormElement& element,
     // field->name() will contain the id only if the name does not exist.  Add
     // an id() method to WebFormControlElement and use that here.
     name_map[field->name()] = field;
+    fields_extracted[i] = true;
   }
 
   // Don't extract field labels if we have no fields.
@@ -161,16 +167,22 @@ bool FormManager::WebFormElementToFormData(const WebFormElement& element,
       iter->second->set_label(label.innerText());
   }
 
-  for (size_t i = 0; i < control_elements.size(); ++i) {
-    const WebFormControlElement& control_element = control_elements[i];
-
-    std::map<string16, FormField*>::iterator iter =
-        name_map.find(control_element.nameForAutofill());
-    if (iter == name_map.end())
+  // Loop through the form control elements, extracting the label text from the
+  // DOM.  We use the |fields_extracted| vector to make sure we assign the
+  // extracted label to the correct field, as it's possible |form_fields| will
+  // not contain all of the elements in |control_elements|.
+  for (size_t i = 0, field_idx = 0;
+       i < control_elements.size() && field_idx < form_fields.size(); ++i) {
+    // This field didn't meet the requirements, so don't try to find a label for
+    // it.
+    if (!fields_extracted[i])
       continue;
 
-    if (iter->second->label().empty())
-      iter->second->set_label(InferLabelForElement(control_element));
+    const WebFormControlElement& control_element = control_elements[i];
+    if (form_fields[field_idx]->label().empty())
+      form_fields[field_idx]->set_label(InferLabelForElement(control_element));
+
+    ++field_idx;
   }
 
   // Copy the created FormFields into the resulting FormData object.
diff --git a/chrome/renderer/form_manager_unittest.cc b/chrome/renderer/form_manager_unittest.cc
index 5c32a76..6d97d23 100644
--- a/chrome/renderer/form_manager_unittest.cc
+++ b/chrome/renderer/form_manager_unittest.cc
@@ -512,6 +512,54 @@ TEST_F(FormManagerTest, InvalidLabels) {
                       fields[2]);
 }
 
+// This test has three form control elements, only one of which has a label
+// element associated with it.  The first element is disabled because of the
+// autocomplete=off attribute.
+TEST_F(FormManagerTest, OneLabelElementFirstControlElementDisabled) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "  First name:"
+           "    <INPUT type=\"text\" id=\"firstname\" autocomplete=\"off\"/>"
+           "  <LABEL for=\"middlename\">Middle name: </LABEL>"
+           "    <INPUT type=\"text\" id=\"middlename\"/>"
+           "  Last name:"
+           "    <INPUT type=\"text\" id=\"lastname\"/>"
+           "  <INPUT type=\"submit\" name=\"reply-send\" value=\"Send\"/>"
+           "</FORM>");
+
+  WebFrame* web_frame = GetMainFrame();
+  ASSERT_NE(static_cast<WebFrame*>(NULL), web_frame);
+
+  FormManager form_manager;
+  form_manager.ExtractForms(web_frame);
+
+  std::vector<FormData> forms;
+  form_manager.GetForms(FormManager::REQUIRE_AUTOCOMPLETE, &forms);
+  ASSERT_EQ(1U, forms.size());
+
+  const FormData& form = forms[0];
+  EXPECT_EQ(ASCIIToUTF16("TestForm"), form.name);
+  EXPECT_EQ(GURL(web_frame->url()), form.origin);
+  EXPECT_EQ(GURL("http://cnn.com"), form.action);
+
+  const std::vector<FormField>& fields = form.fields;
+  ASSERT_EQ(3U, fields.size());
+  EXPECT_EQ(FormField(ASCIIToUTF16("Middle name:"),
+                      ASCIIToUTF16("middlename"),
+                      string16(),
+                      ASCIIToUTF16("text")),
+                      fields[0]);
+  EXPECT_EQ(FormField(ASCIIToUTF16("Last name:"),
+                      ASCIIToUTF16("lastname"),
+                      string16(),
+                      ASCIIToUTF16("text")),
+                      fields[1]);
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("reply-send"),
+                      ASCIIToUTF16("Send"),
+                      ASCIIToUTF16("submit")),
+                      fields[2]);
+}
+
 TEST_F(FormManagerTest, LabelsInferredFromText) {
   LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
            "  First name:"
@@ -652,4 +700,47 @@ TEST_F(FormManagerTest, LabelsInferredFromTableCell) {
                       fields[2]);
 }
 
+TEST_F(FormManagerTest, InferredLabelsWithSameName) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "  Address Line 1:"
+           "    <INPUT type=\"text\" name=\"Address\"/>"
+           "  Address Line 2:"
+           "    <INPUT type=\"text\" name=\"Address\"/>"
+           "  <INPUT type=\"submit\" name=\"reply-send\" value=\"Send\"/>"
+           "</FORM>");
+
+  WebFrame* web_frame = GetMainFrame();
+  ASSERT_NE(static_cast<WebFrame*>(NULL), web_frame);
+
+  FormManager form_manager;
+  form_manager.ExtractForms(web_frame);
+
+  std::vector<FormData> forms;
+  form_manager.GetForms(FormManager::REQUIRE_NONE, &forms);
+  ASSERT_EQ(1U, forms.size());
+
+  const FormData& form = forms[0];
+  EXPECT_EQ(ASCIIToUTF16("TestForm"), form.name);
+  EXPECT_EQ(GURL(web_frame->url()), form.origin);
+  EXPECT_EQ(GURL("http://cnn.com"), form.action);
+
+  const std::vector<FormField>& fields = form.fields;
+  ASSERT_EQ(3U, fields.size());
+  EXPECT_EQ(FormField(ASCIIToUTF16("Address Line 1:"),
+                      ASCIIToUTF16("Address"),
+                      string16(),
+                      ASCIIToUTF16("text")),
+                      fields[0]);
+  EXPECT_EQ(FormField(ASCIIToUTF16("Address Line 2:"),
+                      ASCIIToUTF16("Address"),
+                      string16(),
+                      ASCIIToUTF16("text")),
+                      fields[1]);
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("reply-send"),
+                      ASCIIToUTF16("Send"),
+                      ASCIIToUTF16("submit")),
+                      fields[2]);
+}
+
 }  // namespace