Bug 1439023 - Don't reset autocomplete controller state when autofill fell back to form history. r=jaws
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Fri, 08 Feb 2019 17:46:51 -0800
changeset 458980 047455dcfc20
parent 458979 6d917b008b34
child 458981 3dd87564278e
push id35553
push usershindli@mozilla.com
push dateThu, 14 Feb 2019 04:41:18 +0000
treeherdermozilla-central@f0ea53f47215 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1439023
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1439023 - Don't reset autocomplete controller state when autofill fell back to form history. r=jaws Otherwise the cached value won't be used for filling by satchel and _fillFromAutocompleteRow won't fill a result that isn't a `autofill-profile` style: https://searchfox.org/mozilla-central/rev/03ebbdab952409640c6857d835d3040bf6f9e2db/browser/extensions/formautofill/FormAutofillContent.jsm#283,295 Differential Revision: https://phabricator.services.mozilla.com/D19249
browser/extensions/formautofill/FormAutofillContent.jsm
browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
toolkit/content/widgets/autocomplete.xml
--- a/browser/extensions/formautofill/FormAutofillContent.jsm
+++ b/browser/extensions/formautofill/FormAutofillContent.jsm
@@ -112,27 +112,29 @@ AutofillProfileAutoCompleteSearch.protot
     let isAddressField = FormAutofillUtils.isAddressField(activeFieldDetail.fieldName);
     let isInputAutofilled = activeFieldDetail.state == FIELD_STATES.AUTO_FILLED;
     let allFieldNames = activeSection.allFieldNames;
     let filledRecordGUID = activeSection.filledRecordGUID;
     let searchPermitted = isAddressField ?
                           FormAutofill.isAutofillAddressesEnabled :
                           FormAutofill.isAutofillCreditCardsEnabled;
     let AutocompleteResult = isAddressField ? AddressResult : CreditCardResult;
+    let isFormAutofillSearch = true;
     let pendingSearchResult = null;
 
     ProfileAutocomplete.lastProfileAutoCompleteFocusedInput = activeInput;
     // Fallback to form-history if ...
     //   - specified autofill feature is pref off.
     //   - no profile can fill the currently-focused input.
     //   - the current form has already been populated.
     //   - (address only) less than 3 inputs are covered by all saved fields in the storage.
     if (!searchPermitted || !savedFieldNames.has(activeFieldDetail.fieldName) ||
         (!isInputAutofilled && filledRecordGUID) || (isAddressField &&
         allFieldNames.filter(field => savedFieldNames.has(field)).length < FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD)) {
+      isFormAutofillSearch = false;
       if (activeInput.autocomplete == "off") {
         // Create a dummy result as an empty search result.
         pendingSearchResult = new AutocompleteResult("", "", [], [], {});
       } else {
         pendingSearchResult = new Promise(resolve => {
           let formHistory = Cc["@mozilla.org/autocomplete/search;1?name=form-history"]
                             .createInstance(Ci.nsIAutoCompleteSearch);
           formHistory.startSearch(searchString, searchParam, previousResult, {
@@ -168,21 +170,25 @@ AutofillProfileAutoCompleteSearch.protot
                                       allFieldNames,
                                       adaptedRecords,
                                       {isSecure, isInputAutofilled});
       });
     }
 
     Promise.resolve(pendingSearchResult).then((result) => {
       listener.onSearchResult(this, result);
-      ProfileAutocomplete.lastProfileAutoCompleteResult = result;
-      // Reset AutoCompleteController's state at the end of startSearch to ensure that
-      // none of form autofill result will be cached in other places and make the
-      // result out of sync.
-      autocompleteController.resetInternalState();
+      // Don't save cache results or reset state when returning non-autofill results such as the
+      // form history fallback above.
+      if (isFormAutofillSearch) {
+        ProfileAutocomplete.lastProfileAutoCompleteResult = result;
+        // Reset AutoCompleteController's state at the end of startSearch to ensure that
+        // none of form autofill result will be cached in other places and make the
+        // result out of sync.
+        autocompleteController.resetInternalState();
+      }
     });
   },
 
   /**
    * Stops an asynchronous search that is in progress
    */
   stopSearch() {
     ProfileAutocomplete.lastProfileAutoCompleteResult = null;
--- a/browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
+++ b/browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
@@ -32,17 +32,16 @@ let MOCK_STORAGE = [{
   "address-level1": "CA",
 }];
 
 async function setupAddressStorage() {
   await addAddress(MOCK_STORAGE[0]);
   await addAddress(MOCK_STORAGE[1]);
 }
 
-// TODO: Add form history fallback test cases for credit card fields. (bug 1393374)
 async function setupFormHistory() {
   await updateFormHistory([
     {op: "add", fieldname: "tel", value: "+1234567890"},
     {op: "add", fieldname: "email", value: "foo@mozilla.com"},
   ]);
 }
 
 initPopupListener();
@@ -154,34 +153,36 @@ add_task(async function check_fields_aft
   await triggerAutofillAndCheckProfile(MOCK_STORAGE[1]);
   synthesizeKey("KEY_Escape");
   is(focusedInput.value, "Mozilla", "Filled field shouldn't be reverted by ESC key");
 });
 
 // Fallback to history search after autofill address.
 add_task(async function check_fallback_after_form_autofill() {
   await setInput("#tel", "", true);
-  synthesizeKey("KEY_ArrowDown");
-  await expectPopup();
+  await triggerPopupAndHoverItem("#tel", 0);
   checkMenuEntries(["+1234567890"], false);
+  await triggerAutofillAndCheckProfile({
+    tel: "+1234567890",
+  });
 });
 
 // Resume form autofill once all the autofilled fileds are changed.
 add_task(async function check_form_autofill_resume() {
   document.querySelector("#tel").blur();
   document.querySelector("#form1").reset();
   await setInput("#tel", "");
-  synthesizeKey("KEY_ArrowDown");
-  await expectPopup();
+  await triggerPopupAndHoverItem("#tel", 0);
   checkMenuEntries(MOCK_STORAGE.map(address =>
     JSON.stringify({
       primary: address.tel,
       secondary: FormAutofillUtils.toOneLineAddress(address["street-address"]),
     })
   ));
+  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
 });
 
 </script>
 
 <p id="display"></p>
 
 <div id="content">
 
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -990,17 +990,21 @@
                      this._rlbPadding;
           }
 
           let currentHeight = this.richlistbox.getBoundingClientRect().height;
           if (height <= currentHeight) {
             this._collapseUnusedItems();
           }
           this.richlistbox.style.removeProperty("height");
-          this.richlistbox.height = height;
+          // We need to get the ceiling of the calculated value to ensure that the box fully contains
+          // all of its contents and doesn't cause a scrollbar since nsIBoxObject only expects a
+          // `long`. e.g. if `height` is 99.5 the richlistbox would render at height 99px with a
+          // scrollbar for the extra 0.5px.
+          this.richlistbox.height = Math.ceil(height);
           ]]>
         </body>
       </method>
 
       <method name="_appendCurrentResult">
         <parameter name="invalidateReason"/>
         <body>
           <![CDATA[