AutoFill: Skip past intervening empty TDs when parsing labels.

BUG=57107
TEST=FormStructure.LabelsInferredFromTableEmptyTDs

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

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

2 files changed, 86 insertions, 12 deletions

diff --git a/chrome/renderer/form_manager.cc b/chrome/renderer/form_manager.cc
index 446cbc9..092fa4b 100644
--- a/chrome/renderer/form_manager.cc
+++ b/chrome/renderer/form_manager.cc
@@ -163,22 +163,27 @@ string16 InferLabelFromTable(
          !parent.to<WebElement>().hasTagName("td"))
     parent = parent.parentNode();
 
-  if (!parent.isNull() && parent.isElementNode()) {
-    WebElement element = parent.to<WebElement>();
-    if (element.hasTagName("td")) {
-      WebNode previous = parent.previousSibling();
+  if (parent.isNull() || !parent.isElementNode())
+    return string16();
 
-      // Skip by any intervening text nodes.
-      while (!previous.isNull() && previous.isTextNode())
-        previous = previous.previousSibling();
+  WebElement e = parent.to<WebElement>();
+  if (e.isNull() || !e.hasTagName("td"))
+    return string16();
 
-      if (!previous.isNull() && previous.isElementNode()) {
-        element = previous.to<WebElement>();
-        if (element.hasTagName("td")) {
-          inferred_label = FindChildText(element);
-        }
+  // Check all previous siblings, skipping non-element nodes, until we find a
+  // non-empty text block.
+  WebNode previous = parent.previousSibling();
+  while (!previous.isNull()) {
+    if (previous.isElementNode()) {
+      e = previous.to<WebElement>();
+      if (e.hasTagName("td")) {
+        inferred_label = FindChildText(e);
+        if (!inferred_label.empty())
+          break;
       }
     }
+
+    previous = previous.previousSibling();
   }
 
   return inferred_label;
diff --git a/chrome/renderer/form_manager_browsertest.cc b/chrome/renderer/form_manager_browsertest.cc
index a4fe748..1ee9935 100644
--- a/chrome/renderer/form_manager_browsertest.cc
+++ b/chrome/renderer/form_manager_browsertest.cc
@@ -1116,6 +1116,75 @@ TEST_F(FormManagerTest, LabelsInferredFromTableCellNested) {
             fields[2]);
 }
 
+TEST_F(FormManagerTest, LabelsInferredFromTableEmptyTDs) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "<TABLE>"
+           "  <TR>"
+           "    <TD>"
+           "      <SPAN>*</SPAN>"
+           "      <B>First Name</B>"
+           "    </TD>"
+           "    <TD></TD>"
+           "    <TD>"
+           "      <INPUT type=\"text\" id=\"firstname\" value=\"John\"/>"
+           "    </TD>"
+           "  </TR>"
+           "  <TR>"
+           "    <TD>"
+           "      <SPAN>*</SPAN>"
+           "      <B>Last Name</B>"
+           "    </TD>"
+           "    <TD></TD>"
+           "    <TD>"
+           "      <INPUT type=\"text\" id=\"lastname\" value=\"Milton\"/>"
+           "    </TD>"
+           "  </TR>"
+           "  <TR>"
+           "    <TD></TD>"
+           "    <TD>"
+           "      <INPUT type=\"submit\" name=\"reply-send\" value=\"Send\"/>"
+           "    </TD>"
+           "  </TR>"
+           "</TABLE>"
+           "</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("Milton"),
+                      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>"