Bug 1439023 - Don't reset autocomplete controller state when autofill fell back to form history. r=jaws, a=lizzard
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Fri, 08 Feb 2019 17:46:51 -0800
changeset 513216 c6abdbd324f774a4e3fe0edf7dcb2a5fa03f26eb
parent 513215 8d7fe5194e9399f47dd1a3abe65722c6b03cf78d
child 513217 e4171c06f422b19fa9fb2b80a7b7e802395c2daa
push id10754
push userjcristau@mozilla.com
push dateFri, 22 Feb 2019 11:18:35 +0000
treeherdermozilla-beta@c6abdbd324f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, lizzard
bugs1439023
milestone66.0
Bug 1439023 - Don't reset autocomplete controller state when autofill fell back to form history. r=jaws, a=lizzard 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
@@ -991,17 +991,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[