Do not set autofill property for datalist options

chrome/browser/autofill/autofill_interactive_uitest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/memory/ref_counted.h"
@@ skipping 276 matching lines @@
                         const std::string& expected_value) {
     std::string value;
     ASSERT_TRUE(content::ExecuteScriptAndExtractString(
         GetWebContents(),
         "window.domAutomationController.send("
         "    document.getElementById('" + field_name + "').value);",
         &value));
     EXPECT_EQ(expected_value, value);
   }
 
+  void GetFieldBackgroundColor(const std::string& field_name,
+                               std::string& color) {

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Please pass a pointer.  We pretty much never pass by non-const-reference in
the Chromium repository.

[ziran.sun, 2014/08/26 13:32:15]

Done.

+    ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+        GetWebContents(),
+        "window.domAutomationController.send("
+        "    document.defaultView.getComputedStyle(document.getElementById('" +
+            field_name + "')).backgroundColor);",

[Ilya Sherman, 2014/08/22 23:49:17]

nit: The current indentation looks really odd.  I'd either de-indent this line,
or indent the line above by four spaces.

[ziran.sun, 2014/08/26 13:32:15]

I tried both suggestions, "git cl format" doesn't seem liking them :(. It
returns the original format we have here.

 

[Ilya Sherman, 2014/08/27 00:37:47]

I understand that this is the format that "git cl format" returns, but "git cl
format" is not entirely canonical -- it's a good starting point, but it
sometimes gets the formatting wrong.  This is such a case.

+        &color));
+  }
+
   void SimulateURLFetch(bool success) {
     net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
     ASSERT_TRUE(fetcher);
     net::URLRequestStatus status;
     status.set_status(success ? net::URLRequestStatus::SUCCESS :
                                 net::URLRequestStatus::FAILED);
 
     std::string script = " var google = {};"
         "google.translate = (function() {"
         "  return {"
@@ skipping 109 matching lines @@
     event.type = blink::WebKeyboardEvent::RawKeyDown;
     test_delegate_.Reset();
     // Install the key press event sink to ensure that any events that are not
     // handled by the installed callbacks do not end up crashing the test.
     GetRenderViewHost()->AddKeyPressEventCallback(key_press_event_sink_);
     GetRenderViewHost()->ForwardKeyboardEvent(event);
     test_delegate_.Wait();
     GetRenderViewHost()->RemoveKeyPressEventCallback(key_press_event_sink_);
   }
 
+  // There is no need to start message loop for Datalist.

[Ilya Sherman, 2014/08/22 23:49:17]

Hmm, why not?  Doesn't it use the same UI?

Even if it's not *necessary* to start the message loop, does it hurt anything to
do so?  If not, I'd prefer that we reuse the SendKeyToPopupAndWait() method, for
the sake of having less (and therefore simpler) code. 

[ziran.sun, 2014/08/26 13:32:15]

Existing test cases in this test file assume that once select an option in the
suggestion popup box, the selection gets previewed. So after
"DidSelectSuggestion()" is called, "DidPreviewFormData()" should be called.
"DidPreviewFormData()" defined in this file will stop the message loop.

We don't have preview support for datalist. If we starts message loop for
datalist, after "DidSelectSuggestion()" it will wait for autofill preview that
will never happen. This will cause the message loop hang and the test will end
up with timeout.

[Ilya Sherman, 2014/08/27 00:37:47]

I see.  That makes sense; thanks for the explanation.

+  void SendKeyToDataListPopup(ui::KeyboardCode key) {
+    // Route popup-targeted key presses via the render view host.
+    content::NativeWebKeyboardEvent event;
+    event.windowsKeyCode = key;
+    event.type = blink::WebKeyboardEvent::RawKeyDown;
+    // Install the key press event sink to ensure that any events that are not
+    // handled by the installed callbacks do not end up crashing the test.
+    GetRenderViewHost()->AddKeyPressEventCallback(key_press_event_sink_);
+    GetRenderViewHost()->ForwardKeyboardEvent(event);
+    GetRenderViewHost()->RemoveKeyPressEventCallback(key_press_event_sink_);
+  }
+
   void TryBasicFormFill() {
     FocusFirstNameField();
 
     // Start filling the first name field with "M" and wait for the popup to be
     // shown.
     SendKeyToPageAndWait(ui::VKEY_M);
 
     // Press the down arrow to select the suggestion and preview the autofilled
     // form.
     SendKeyToPopupAndWait(ui::VKEY_DOWN);
@@ skipping 191 matching lines @@
       "document.getElementById('firstname').value = '';"));
   ExpectFieldValue("firstname", "");
 
   // Invoke and accept the Autofill popup and verify the field was filled.
   SendKeyToPageAndWait(ui::VKEY_M);
   SendKeyToPopupAndWait(ui::VKEY_DOWN);
   SendKeyToPopupAndWait(ui::VKEY_RETURN);
   ExpectFieldValue("firstname", "Milton");
 }
 
+// Test that an input field should not be autofilled when choose an option from
+// its datalist option list.

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Perhaps something like "Test that an input field is not rendered with the
yellow autofilled background color when choosing an option from the datalist
suggestion list."? (As always, feel free to refine further :)

[ziran.sun, 2014/08/26 13:32:15]

Done.

+IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnSelectOptionFromDatalist) {
+  // Load the test page.
+  ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(
+      browser(),
+      GURL(std::string(kDataURIPrefix) +
+           "<form action=\"http://www.example.com/\" method=\"POST\">"
+           " <input list=\"dl\" type=\"search\" id=\"firstname\""

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Please indent this line, and the ones below, by one additional space
(within the quotation marks).  That is, the start of the line should be

           "  <input ...

[ziran.sun, 2014/08/26 13:32:15]

Done.

+           "        onfocus=\"domAutomationController.send(true)\"><br>"
+           " <datalist id=\"dl\">"

[Ilya Sherman, 2014/08/22 23:49:17]

nit: Why does the datalist need an id?  Would the test work fine if you omitted
this?

[ziran.sun, 2014/08/26 13:32:15]

I think the minimum argument for datalist tag is to have an id. And we refer
this id in input "list". 

The test doesn't work without id specified here.

+           " <option value=\"Adam\"></option>"
+           " <option value=\"Bob\"></option>"
+           " <option value=\"Carl\"></option>"
+           " </datalist>"
+           "</form>")));
+  std::string orginalcolor;
+  GetFieldBackgroundColor("firstname", orginalcolor);
+
+  FocusFirstNameField();
+  SendKeyToPageAndWait(ui::VKEY_DOWN);
+  SendKeyToDataListPopup(ui::VKEY_DOWN);
+  SendKeyToDataListPopup(ui::VKEY_RETURN);
+  ExpectFieldValue("firstname", "Adam");
+  std::string color;
+  GetFieldBackgroundColor("firstname", color);
+  EXPECT_EQ(color, orginalcolor);
+}
+
 // Test that a JavaScript oninput event is fired after auto-filling a form.
 IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, OnInputAfterAutofill) {
   CreateTestProfile();
 
   const char kOnInputScript[] =
       "<script>"
       "focused_fired = false;"
       "unfocused_fired = false;"
       "changed_select_fired = false;"
       "unchanged_select_fired = false;"
@@ skipping 741 matching lines @@
   ASSERT_TRUE(content::ExecuteScript(
       GetRenderViewHost(),
       "document.querySelector('input').autocomplete = 'off';"));
 
   // Press the down arrow to select the suggestion and attempt to preview the
   // autofilled form.
   SendKeyToPopupAndWait(ui::VKEY_DOWN);
 }
 
 }  // namespace autofill