AutoFill: Parse the previous label element for label text.

BUG=61444
TEST=FormManagerTest.LabelsInferredFromTableLabels

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

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

2 files changed, 134 insertions, 51 deletions

diff --git a/chrome/renderer/form_manager.cc b/chrome/renderer/form_manager.cc
index 265d0c2..5287d61 100644
--- a/chrome/renderer/form_manager.cc
+++ b/chrome/renderer/form_manager.cc
@@ -97,54 +97,76 @@ string16 InferLabelFromPrevious(
     const WebFormControlElement& element) {
   string16 inferred_label;
   WebNode previous = element.previousSibling();
-  if (!previous.isNull()) {
-    // Eg. Some Text<input ...>
-    if (previous.isTextNode()) {
-      inferred_label = previous.nodeValue();
-      TrimWhitespace(inferred_label, TRIM_ALL, &inferred_label);
+  if (previous.isNull())
+    return string16();
+
+  if (previous.isTextNode()) {
+    inferred_label = previous.nodeValue();
+    TrimWhitespace(inferred_label, TRIM_ALL, &inferred_label);
+  }
+
+  // If we didn't find text, check for previous paragraph.
+  // Eg. <p>Some Text</p><input ...>
+  // Note the lack of whitespace between <p> and <input> elements.
+  if (inferred_label.empty()) {
+    if (previous.isElementNode()) {
+      WebElement element = previous.to<WebElement>();
+      if (element.hasTagName("p")) {
+        inferred_label = FindChildText(element);
+      }
     }
+  }
 
-    // If we didn't find text, check for previous paragraph.
-    // Eg. <p>Some Text</p><input ...>
-    // Note the lack of whitespace between <p> and <input> elements.
-    if (inferred_label.empty()) {
-      if (previous.isElementNode()) {
-        WebElement element = previous.to<WebElement>();
-        if (element.hasTagName("p")) {
-          inferred_label = FindChildText(element);
-        }
+  // If we didn't find paragraph, check for previous paragraph to this.
+  // Eg. <p>Some Text</p>   <input ...>
+  // Note the whitespace between <p> and <input> elements.
+  if (inferred_label.empty()) {
+    previous = previous.previousSibling();
+    if (!previous.isNull() && previous.isElementNode()) {
+      WebElement element = previous.to<WebElement>();
+      if (element.hasTagName("p")) {
+        inferred_label = FindChildText(element);
       }
     }
+  }
 
-    // If we didn't find paragraph, check for previous paragraph to this.
-    // Eg. <p>Some Text</p>   <input ...>
-    // Note the whitespace between <p> and <input> elements.
-    if (inferred_label.empty()) {
-      previous = previous.previousSibling();
-      if (!previous.isNull() && previous.isElementNode()) {
+  // Look for text node prior to <img> tag.
+  // Eg. Some Text<img/><input ...>
+  if (inferred_label.empty()) {
+    while (inferred_label.empty() && !previous.isNull()) {
+      if (previous.isTextNode()) {
+        inferred_label = previous.nodeValue();
+        TrimWhitespace(inferred_label, TRIM_ALL, &inferred_label);
+      } else if (previous.isElementNode()) {
         WebElement element = previous.to<WebElement>();
-        if (element.hasTagName("p")) {
-          inferred_label = FindChildText(element);
-        }
+        if (!element.hasTagName("img"))
+          break;
+      } else {
+        break;
       }
+      previous = previous.previousSibling();
     }
+  }
 
-    // Look for text node prior to <img> tag.
-    // Eg. Some Text<img/><input ...>
-    if (inferred_label.empty()) {
-      while (inferred_label.empty() && !previous.isNull()) {
-        if (previous.isTextNode()) {
-          inferred_label = previous.nodeValue();
-          TrimWhitespace(inferred_label, TRIM_ALL, &inferred_label);
-        } else if (previous.isElementNode()) {
-          WebElement element = previous.to<WebElement>();
-          if (!element.hasTagName("img"))
-            break;
+  // Look for label node prior to <input> tag.
+  // Eg. <label>Some Text</label><input ...>
+  if (inferred_label.empty()) {
+    while (inferred_label.empty() && !previous.isNull()) {
+      if (previous.isTextNode()) {
+        inferred_label = previous.nodeValue();
+        TrimWhitespace(inferred_label, TRIM_ALL, &inferred_label);
+      } else if (previous.isElementNode()) {
+        WebElement element = previous.to<WebElement>();
+        if (element.hasTagName("label")) {
+          inferred_label = FindChildText(element);
         } else {
           break;
         }
-        previous = previous.previousSibling();
+      } else {
+        break;
       }
+
+      previous = previous.previousSibling();
     }
   }
 
diff --git a/chrome/renderer/form_manager_browsertest.cc b/chrome/renderer/form_manager_browsertest.cc
index 76ff574..3f3e6ad 100644
--- a/chrome/renderer/form_manager_browsertest.cc
+++ b/chrome/renderer/form_manager_browsertest.cc
@@ -755,7 +755,9 @@ TEST_F(FormManagerTest, LabelsWithSpans) {
 // This test is different from FormManagerTest.Labels in that the label elements
 // for= attribute is set to the name of the form control element it is a label
 // for instead of the id of the form control element.  This is invalid because
-// the for= attribute must be set to the id of the form control element.
+// the for= attribute must be set to the id of the form control element;
+// however, current label parsing code will extract the text from the previous
+// label element and apply it to the following input field.
 TEST_F(FormManagerTest, InvalidLabels) {
   LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
            "  <LABEL for=\"firstname\"> First name: </LABEL>"
@@ -782,21 +784,24 @@ TEST_F(FormManagerTest, InvalidLabels) {
 
   const std::vector<FormField>& fields = form.fields;
   ASSERT_EQ(3U, fields.size());
-  EXPECT_TRUE(fields[0].StrictlyEqualsHack(FormField(string16(),
-                                           ASCIIToUTF16("firstname"),
-                                           ASCIIToUTF16("John"),
-                                           ASCIIToUTF16("text"),
-                                           20)));
-  EXPECT_TRUE(fields[1].StrictlyEqualsHack(FormField(string16(),
-                                           ASCIIToUTF16("lastname"),
-                                           ASCIIToUTF16("Smith"),
-                                           ASCIIToUTF16("text"),
-                                           20)));
-  EXPECT_TRUE(fields[2].StrictlyEqualsHack(FormField(string16(),
-                                           ASCIIToUTF16("reply-send"),
-                                           string16(),
-                                           ASCIIToUTF16("submit"),
-                                           0)));
+  EXPECT_TRUE(fields[0].StrictlyEqualsHack(
+      FormField(ASCIIToUTF16("First name:"),
+                ASCIIToUTF16("firstname"),
+                ASCIIToUTF16("John"),
+                ASCIIToUTF16("text"),
+                20)));
+  EXPECT_TRUE(fields[1].StrictlyEqualsHack(
+      FormField(ASCIIToUTF16("Last name:"),
+                ASCIIToUTF16("lastname"),
+                ASCIIToUTF16("Smith"),
+                ASCIIToUTF16("text"),
+                20)));
+  EXPECT_TRUE(fields[2].StrictlyEqualsHack(
+      FormField(string16(),
+                ASCIIToUTF16("reply-send"),
+                string16(),
+                ASCIIToUTF16("submit"),
+                0)));
 }
 
 // This test has three form control elements, only one of which has a label
@@ -1188,6 +1193,62 @@ TEST_F(FormManagerTest, LabelsInferredFromTableEmptyTDs) {
             fields[2]);
 }
 
+TEST_F(FormManagerTest, LabelsInferredFromTableLabels) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "<TABLE>"
+           "  <TR>"
+           "    <TD>"
+           "      <LABEL>First Name:</LABEL>"
+           "      <INPUT type=\"text\" id=\"firstname\" value=\"John\"/>"
+           "    </TD>"
+           "  </TR>"
+           "  <TR>"
+           "    <TD>"
+           "      <LABEL>Last Name:</LABEL>"
+           "      <INPUT type=\"text\" id=\"lastname\" value=\"Smith\"/>"
+           "    </TD>"
+           "  </TR>"
+           "</TABLE>"
+           "<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.GetFormsInFrame(web_frame, 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("First Name:"),
+                      ASCIIToUTF16("firstname"),
+                      ASCIIToUTF16("John"),
+                      ASCIIToUTF16("text"),
+                      20),
+            fields[0]);
+  EXPECT_EQ(FormField(ASCIIToUTF16("Last Name:"),
+                      ASCIIToUTF16("lastname"),
+                      ASCIIToUTF16("Smith"),
+                      ASCIIToUTF16("text"),
+                      20),
+            fields[1]);
+  EXPECT_EQ(FormField(string16(),
+                      ASCIIToUTF16("reply-send"),
+                      ASCIIToUTF16("Send"),
+                      ASCIIToUTF16("submit"),
+                      0),
+            fields[2]);
+}
+
 TEST_F(FormManagerTest, LabelsInferredFromDefinitionList) {
   LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
            "<DL>"