author | steveck-chung <schung@mozilla.com> |
Fri, 01 Sep 2017 10:11:19 +0800 | |
changeset 659027 | d3e60458037a514ec247b30ece2b955c35a73de9 |
parent 656346 | 04b6be50a2526c7a26a63715f441c47e1aa1f9be |
child 729856 | 4d9febdc7c5362095c52dcf0a4ea6cb781081564 |
push id | 77986 |
push user | bmo:schung@mozilla.com |
push date | Tue, 05 Sep 2017 10:35:26 +0000 |
reviewers | lchang |
bugs | 1395519 |
milestone | 57.0a1 |
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm +++ b/browser/extensions/formautofill/FormAutofillHandler.jsm @@ -531,37 +531,42 @@ FormAutofillHandler.prototype = { // Try to abbreviate the value of select element. if (type == "address" && detail.fieldName == "address-level1" && element instanceof Ci.nsIDOMHTMLSelectElement) { // Don't save the record when the option value is empty *OR* there // are multiple options being selected. The empty option is usually // assumed to be default along with a meaningless text to users. if (!value || element.selectedOptions.length != 1) { + // Keep the property and preserve more information for address updating + data[type].record[detail.fieldName] = ""; return; } let text = element.selectedOptions[0].text.trim(); value = FormAutofillUtils.getAbbreviatedStateName([value, text]) || text; } if (!value) { + // Keep the property and preserve more information for updating + data[type].record[detail.fieldName] = ""; return; } data[type].record[detail.fieldName] = value; if (detail.state == "AUTO_FILLED") { data[type].untouchedFields.push(detail.fieldName); } }); }); if (data.address && - Object.keys(data.address.record).length < FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD) { + Object.values(data.address.record).filter(v => v).length < + FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD) { log.debug("No address record saving since there are only", Object.keys(data.address.record).length, "usable fields"); delete data.address; } if (data.creditCard && (!data.creditCard.record["cc-number"] || !FormAutofillUtils.isCCNumber(data.creditCard.record["cc-number"]))) {
--- a/browser/extensions/formautofill/FormAutofillParent.jsm +++ b/browser/extensions/formautofill/FormAutofillParent.jsm @@ -371,17 +371,17 @@ FormAutofillParent.prototype = { switch (state) { case "create": if (!changedGUIDs.length) { changedGUIDs.push(this.profileStorage.addresses.add(address.record)); } break; case "update": if (!changedGUIDs.length) { - this.profileStorage.addresses.update(address.guid, address.record); + this.profileStorage.addresses.update(address.guid, address.record, true); changedGUIDs.push(address.guid); } else { this.profileStorage.addresses.remove(address.guid); } break; } changedGUIDs.forEach(guid => this.profileStorage.addresses.notifyUsed(guid)); });
--- a/browser/extensions/formautofill/ProfileStorage.jsm +++ b/browser/extensions/formautofill/ProfileStorage.jsm @@ -369,36 +369,44 @@ class AutofillRecords { /** * Update the specified record. * * @param {string} guid * Indicates which record to update. * @param {Object} record * The new record used to overwrite the old one. + * @param {boolean} [preserveOldProperties = false] + * Preserve old record's properties if they don't exist in new record. */ - update(guid, record) { + update(guid, record, preserveOldProperties = false) { this.log.debug("update:", guid, record); let recordFound = this._findByGUID(guid); if (!recordFound) { throw new Error("No matching record."); } - let recordToUpdate = this._clone(record); + // Clone the record by Object assign API to preserve the property with empty string. + let recordToUpdate = Object.assign({}, record); this._normalizeRecord(recordToUpdate); for (let field of this.VALID_FIELDS) { let oldValue = recordFound[field]; let newValue = recordToUpdate[field]; - if (newValue != null) { + // Resume the old field value in the perserve case + if (preserveOldProperties && newValue === undefined) { + newValue = oldValue; + } + + if (!newValue) { + delete recordFound[field]; + } else { recordFound[field] = newValue; - } else { - delete recordFound[field]; } this._maybeStoreLastSyncedField(recordFound, field, oldValue); } recordFound.timeLastModified = Date.now(); let syncMetadata = this._getSyncMetaData(recordFound); if (syncMetadata) {
--- a/browser/extensions/formautofill/test/browser/browser.ini +++ b/browser/extensions/formautofill/test/browser/browser.ini @@ -1,13 +1,14 @@ [DEFAULT] head = head.js support-files = ../fixtures/autocomplete_basic.html + ../fixtures/autocomplete_simple_basic.html ../fixtures/autocomplete_creditcard_basic.html [browser_autocomplete_footer.js] [browser_autocomplete_marked_back_forward.js] [browser_autocomplete_marked_detached_tab.js] [browser_check_installed.js] [browser_creditCard_doorhanger.js] [browser_dropdown_layout.js]
--- a/browser/extensions/formautofill/test/browser/browser_update_doorhanger.js +++ b/browser/extensions/formautofill/test/browser/browser_update_doorhanger.js @@ -132,8 +132,43 @@ add_task(async function test_submit_unto } ); addresses = await getAddresses(); is(addresses.length, 2, "Still 2 addresses in storage"); is(addresses[0].organization, "Organization", "organization should change"); is(addresses[0].tel, "+16172535702", "tel should remain unchanged"); }); + +add_task(async function test_submit_reduced_fields() { + let addresses = await getAddresses(); + is(addresses.length, 2, "2 addresses in storage"); + + let url = BASE_URL + "autocomplete_simple_basic.html"; + await BrowserTestUtils.withNewTab({gBrowser, url}, + async function(browser) { + let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel, + "popupshown"); + await openPopupOn(browser, "form#simple input[name=tel]"); + await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser); + await BrowserTestUtils.synthesizeKey("VK_RETURN", {}, browser); + + await ContentTask.spawn(browser, null, async function() { + let form = content.document.querySelector("form#simple"); + let tel = form.querySelector("input[name=tel]"); + await new Promise(resolve => setTimeout(resolve, 1000)); + tel.setUserInput("123456789"); + + // Wait 1000ms before submission to make sure the input value applied + await new Promise(resolve => setTimeout(resolve, 1000)); + form.querySelector("input[type=submit]").click(); + }); + + await promiseShown; + await clickDoorhangerButton(MAIN_BUTTON); + } + ); + + addresses = await getAddresses(); + is(addresses.length, 2, "Still 2 addresses in storage"); + is(addresses[0].tel, "123456789", "tel should should be changed"); + is(addresses[0]["postal-code"], "02139", "postal code should be kept"); +});
new file mode 100644 --- /dev/null +++ b/browser/extensions/formautofill/test/fixtures/autocomplete_simple_basic.html @@ -0,0 +1,19 @@ +<!DOCTYPE html> +<html> +<head> + <meta charset="utf-8"> + <title>Form Autofill Demo Page for Simplified Form Case</title> +</head> +<body> + <h1>Form Autofill Demo Page for Simplified Form Case</h1> + + <form id="simple"> + <p><label>Organization: <input type="text" /></label></p> + <p><label>Telephone: <input id="simple_tel" name="tel" /></label></p> + <p><label>Email: <input type="text" id="simple_email" name="email" /></label></p> + <p><input type="submit" /></p> + <p><button type="reset">Reset</button></p> + </form> + +</body> +</html>
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js +++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js @@ -21,27 +21,34 @@ const TEST_ADDRESS_1 = { }; const TEST_ADDRESS_2 = { "street-address": "Some Address", country: "US", }; const TEST_ADDRESS_3 = { + "given-name": "Timothy", + "family-name": "Berners-Lee", "street-address": "Other Address", "postal-code": "12345", }; const TEST_ADDRESS_4 = { "given-name": "Timothy", "additional-name": "John", "family-name": "Berners-Lee", organization: "World Wide Web Consortium", }; +const TEST_ADDRESS_FOR_UPDATE = { + "name": "Tim Berners", + "street-address": "", +}; + const TEST_ADDRESS_WITH_INVALID_FIELD = { "street-address": "Another Address", invalidField: "INVALID", }; const MERGE_TESTCASES = [ { description: "Merge a superset", @@ -317,16 +324,32 @@ add_task(async function test_update() { let address = profileStorage.addresses.get(guid, {rawData: true}); do_check_eq(address.country, undefined); do_check_neq(address.timeLastModified, timeLastModified); do_check_record_matches(address, TEST_ADDRESS_3); do_check_eq(getSyncChangeCounter(profileStorage.addresses, guid), 1); + // Test preserveOldProperties parameter and field with empty string. + profileStorage.addresses.update(guid, TEST_ADDRESS_FOR_UPDATE, true); + await onChanged; + await profileStorage._saveImmediately(); + + profileStorage.addresses.pullSyncChanges(); // force sync metadata, which we check below. + + address = profileStorage.addresses.get(guid, {rawData: true}); + + do_check_eq(address["given-name"], "Tim"); + do_check_eq(address["family-name"], "Berners"); + do_check_eq(address["street-address"], undefined); + do_check_eq(address["postal-code"], "12345"); + do_check_neq(address.timeLastModified, timeLastModified); + do_check_eq(getSyncChangeCounter(profileStorage.addresses, guid), 2); + Assert.throws( () => profileStorage.addresses.update("INVALID_GUID", TEST_ADDRESS_3), /No matching record\./ ); Assert.throws( () => profileStorage.addresses.update(guid, TEST_ADDRESS_WITH_INVALID_FIELD), /"invalidField" is not a valid field\./
--- a/browser/extensions/formautofill/test/unit/test_onFormSubmitted.js +++ b/browser/extensions/formautofill/test/unit/test_onFormSubmitted.js @@ -61,17 +61,20 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "street-address": "331 E. Evelyn Avenue", + "address-level1": "", + "address-level2": "", "country": "USA", + "email": "", "tel": "1-650-903-0800", }, untouchedFields: [], }, }, }, }, { @@ -111,17 +114,20 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "street-address": "331 E. Evelyn Avenue", + "address-level1": "", + "address-level2": "", "country": "USA", + "email": "", "tel": "1-650-903-0800", }, untouchedFields: [], }, creditCard: { guid: null, record: { "cc-name": "John Doe", @@ -143,17 +149,20 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "street-address": "331 E. Evelyn Avenue", + "address-level1": "", + "address-level2": "", "country": "USA", + "email": "", "tel": "1-650-903-0800", }, untouchedFields: [], }, }, }, }, { @@ -166,17 +175,20 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "street-address": "331 E. Evelyn Avenue", + "address-level1": "", + "address-level2": "", "country": "USA", + "email": "", "tel": "1-650-903-0800", }, untouchedFields: [], }, }, }, }, { @@ -188,18 +200,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "address-level1": "CA", + "address-level2": "", "street-address": "331 E. Evelyn Avenue", "country": "USA", + "email": "", + "tel": "", }, untouchedFields: [], }, }, }, }, { description: "Save state with lowercase value", @@ -210,18 +225,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "address-level1": "CA", + "address-level2": "", "street-address": "331 E. Evelyn Avenue", "country": "USA", + "email": "", + "tel": "", }, untouchedFields: [], }, }, }, }, { description: "Save state with a country code prefixed to the label", @@ -232,18 +250,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "address-level1": "AR", + "address-level2": "", "street-address": "331 E. Evelyn Avenue", "country": "USA", + "email": "", + "tel": "", }, untouchedFields: [], }, }, }, }, { description: "Save state with a country code prefixed to the value", @@ -254,18 +275,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "address-level1": "CA", + "address-level2": "", "street-address": "331 E. Evelyn Avenue", "country": "USA", + "email": "", + "tel": "", }, untouchedFields: [], }, }, }, }, { description: "Save state with a country code prefixed to the value and label", @@ -276,18 +300,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "address-level1": "AZ", + "address-level2": "", "street-address": "331 E. Evelyn Avenue", "country": "USA", + "email": "", + "tel": "", }, untouchedFields: [], }, }, }, }, { description: "Should save select label instead when failed to abbreviate the value", @@ -298,18 +325,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "address-level1": "Arizonac", + "address-level2": "", "street-address": "331 E. Evelyn Avenue", "country": "USA", + "email": "", + "tel": "", }, untouchedFields: [], }, }, }, }, { description: "Shouldn't save select with multiple selections", @@ -321,18 +351,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "street-address": "331 E. Evelyn Avenue", + "address-level1": "", + "address-level2": "", "country": "USA", "tel": "1-650-903-0800", + "email": "", }, untouchedFields: [], }, }, }, }, { description: "Shouldn't save select with empty value", @@ -344,18 +377,21 @@ const TESTCASES = [ }, expectedResult: { formSubmission: true, records: { address: { guid: null, record: { "street-address": "331 E. Evelyn Avenue", + "address-level1": "", + "address-level2": "", "country": "USA", "tel": "1-650-903-0800", + "email": "", }, untouchedFields: [], }, }, }, }, ];