iOS full-form Autofill: Handle partially filled forms correctly.

components/autofill/ios/browser/resources/autofill_controller.js

Index: components/autofill/ios/browser/resources/autofill_controller.js
diff --git a/components/autofill/ios/browser/resources/autofill_controller.js b/components/autofill/ios/browser/resources/autofill_controller.js
index fc26586e421adbd0b3fafbe55f21276cdc756cec..70e36e8c7f7517884e7fe045e077bc16e0cf0c1f 100644
--- a/components/autofill/ios/browser/resources/autofill_controller.js
+++ b/components/autofill/ios/browser/resources/autofill_controller.js
@@ -501,9 +501,14 @@ __gCrWeb.autofill['fillActiveFormField'] = function(data) {
  * Fills a number of fields in the same named form.
  *
  * @param {Object<AutofillFormData>} data The data to fill in.
+ * @param {boolean} onlyFillEmpty Only fill empty fields. Otherwise fill all
+ *     fields.
+ * @param {string} forceFillFieldName Named field will always be filled,

[Justin Donnelly, 2015/07/20 17:21:41]

Is it possible to use document.activeElement rather than having to pass this in?

[bondd, 2015/07/20 23:55:29]

fillActiveFormField() (the function immediately above this one) does use
document.activeElement for single-field Autofill, so it is possible. However, it
needs to do some extra logic involving lastActiveElement and nameForAutofill()
to make sure it has the right field, and it passes in the field name anyway to
check that it matches. I believe this is because there are several async calls
with callbacks involved in the iOS Autofill flow, so it's not 100% guaranteed
that activeElement won't change at an unfortunate time. It seems simpler and
more reliable to pass in the known good forceFillFieldName rather than trying to
involve the possibly flaky document.activeElement.

+ *     regardless of value of |onlyFillEmpty|. May be null.
  * @param {boolean} styleElements Apply Autofill CSS style to filled elements.
  */
-__gCrWeb.autofill['fillForm'] = function(data, styleElements) {
+__gCrWeb.autofill['fillForm'] = function(data, onlyFillEmpty,
+    forceFillFieldName, styleElements) {
   // Inject CSS to style the autofilled elements with a yellow background.
   if (styleElements && !__gCrWeb.autofill.styleInjected) {
     var style = document.createElement('style');
@@ -519,6 +524,7 @@ __gCrWeb.autofill['fillForm'] = function(data, styleElements) {
   // Remove Autofill styling when control element is edited.
   var controlElementInputListener = function(evt) {
     evt.target.removeAttribute('chrome-autofilled');
+    evt.target.isAutofilled = false;
     evt.target.removeEventListener('input', controlElementInputListener);
   };
 
@@ -529,11 +535,21 @@ __gCrWeb.autofill['fillForm'] = function(data, styleElements) {
     if (!__gCrWeb.autofill.isAutofillableElement(element)) {
       continue;
     }
-    var value = data.fields[__gCrWeb['common'].nameForAutofill(element)];
+    var fieldName = __gCrWeb['common'].nameForAutofill(element);
+
+    // 'select-one' elements are autofilled even if non-empty. See

[Justin Donnelly, 2015/07/20 17:21:41]

This comment looks like it's meant to describe the entire if condition. Can you
expand it to be something like. "If directed, skip empty fields unless this is
the forceFillName or it's a 'select-one element See [...]."

[bondd, 2015/07/20 23:55:29]

Done.

+    // AutofillManager::FillOrPreviewDataModelForm().
+    if (onlyFillEmpty && element.value && element.value.length > 0 &&
+        !__gCrWeb.autofill.isSelectElement(element) &&
+        fieldName !== forceFillFieldName) {
+      continue;
+    }
+    var value = data.fields[fieldName];
     if (value) {
       element.value = value;
       if (styleElements) {
         element.setAttribute('chrome-autofilled');
+        element.isAutofilled = true;
         element.addEventListener('input', controlElementInputListener);
       }
     }
@@ -547,6 +563,7 @@ __gCrWeb.autofill['fillForm'] = function(data, styleElements) {
       var controlElements = __gCrWeb.common.getFormControlElements(evt.target);
       for (var i = 0; i < controlElements.length; ++i) {
         controlElements[i].removeAttribute('chrome-autofilled');
+        controlElements[i].isAutofilled = false;
       }
       evt.target.removeEventListener('reset', formResetListener);
     };