author | Luke Chang <lchang@mozilla.com> |
Wed, 01 Nov 2017 19:03:50 +0800 | |
changeset 440601 | 57072e6ba78b252aeb7fbba3d6b9f4dcd55a66bb |
parent 440600 | b23c3a2f8e9d752be559f568ec8168b1ecf5577c |
child 440602 | e1a4d0b5eb152db7d06ccd20f919879a440ab6db |
push id | 8118 |
push user | ryanvm@gmail.com |
push date | Fri, 03 Nov 2017 00:38:34 +0000 |
treeherder | mozilla-beta@1c336e874ae8 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | steveck |
bugs | 1413479 |
milestone | 58.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
|
--- a/browser/extensions/formautofill/ProfileStorage.jsm +++ b/browser/extensions/formautofill/ProfileStorage.jsm @@ -308,52 +308,47 @@ class AutofillRecords { * @param {boolean} [options.sourceSync = false] * Did sync generate this addition? * @returns {string} * The GUID of the newly added item.. */ add(record, {sourceSync = false} = {}) { this.log.debug("add:", record); + let recordToSave = this._cloneAndCleanUp(record); + if (sourceSync) { // Remove tombstones for incoming items that were changed on another // device. Local deletions always lose to avoid data loss. - let index = this._findIndexByGUID(record.guid, { + let index = this._findIndexByGUID(recordToSave.guid, { includeDeleted: true, }); if (index > -1) { let existing = this.data[index]; if (existing.deleted) { this.data.splice(index, 1); } else { - throw new Error(`Record ${record.guid} already exists`); + throw new Error(`Record ${recordToSave.guid} already exists`); } } - let recordToSave = this._clone(record); - return this._saveRecord(recordToSave, {sourceSync}); - } + } else if (!recordToSave.deleted) { + this._normalizeRecord(recordToSave); + + recordToSave.guid = this._generateGUID(); + recordToSave.version = this.version; - if (record.deleted) { - return this._saveRecord(record); + // Metadata + let now = Date.now(); + recordToSave.timeCreated = now; + recordToSave.timeLastModified = now; + recordToSave.timeLastUsed = 0; + recordToSave.timesUsed = 0; } - let recordToSave = this._clone(record); - this._normalizeRecord(recordToSave); - - recordToSave.guid = this._generateGUID(); - recordToSave.version = this.version; - - // Metadata - let now = Date.now(); - recordToSave.timeCreated = now; - recordToSave.timeLastModified = now; - recordToSave.timeLastUsed = 0; - recordToSave.timesUsed = 0; - - return this._saveRecord(recordToSave); + return this._saveRecord(recordToSave, {sourceSync}); } _saveRecord(record, {sourceSync = false} = {}) { if (!record.guid) { throw new Error("Record missing GUID"); } let recordToSave; @@ -423,17 +418,17 @@ class AutofillRecords { let oldValue = recordFound[field]; let newValue = recordToUpdate[field]; // Resume the old field value in the perserve case if (preserveOldProperties && newValue === undefined) { newValue = oldValue; } - if (!newValue) { + if (newValue === undefined || newValue === "") { delete recordFound[field]; } else { recordFound[field] = newValue; } this._maybeStoreLastSyncedField(recordFound, field, oldValue); }
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js +++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js @@ -34,17 +34,17 @@ const TEST_ADDRESS_3 = { const TEST_ADDRESS_4 = { "given-name": "Timothy", "additional-name": "John", "family-name": "Berners-Lee", organization: "World Wide Web Consortium", }; -const TEST_ADDRESS_FOR_UPDATE = { +const TEST_ADDRESS_WITH_EMPTY_FIELD = { "name": "Tim Berners", "street-address": "", }; const TEST_ADDRESS_WITH_INVALID_FIELD = { "street-address": "Another Address", invalidField: "INVALID", }; @@ -294,16 +294,22 @@ add_task(async function test_add() { do_check_neq(addresses[0].guid, undefined); do_check_eq(addresses[0].version, 1); do_check_neq(addresses[0].timeCreated, undefined); do_check_eq(addresses[0].timeLastModified, addresses[0].timeCreated); do_check_eq(addresses[0].timeLastUsed, 0); do_check_eq(addresses[0].timesUsed, 0); + // Empty string should be deleted before saving. + profileStorage.addresses.add(TEST_ADDRESS_WITH_EMPTY_FIELD); + let address = profileStorage.addresses.data[2]; + do_check_eq(address.name, TEST_ADDRESS_WITH_EMPTY_FIELD.name); + do_check_eq(address["street-address"], undefined); + Assert.throws(() => profileStorage.addresses.add(TEST_ADDRESS_WITH_INVALID_FIELD), /"invalidField" is not a valid field\./); }); add_task(async function test_update() { let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, [TEST_ADDRESS_1, TEST_ADDRESS_2]); @@ -328,31 +334,37 @@ 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); + profileStorage.addresses.update(guid, TEST_ADDRESS_WITH_EMPTY_FIELD, 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); + // Empty string should be deleted while updating. + profileStorage.addresses.update(profileStorage.addresses.data[0].guid, TEST_ADDRESS_WITH_EMPTY_FIELD); + address = profileStorage.addresses.data[0]; + do_check_eq(address.name, TEST_ADDRESS_WITH_EMPTY_FIELD.name); + do_check_eq(address["street-address"], undefined); + 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_creditCardRecords.js +++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js @@ -23,16 +23,22 @@ const TEST_CREDIT_CARD_2 = { }; const TEST_CREDIT_CARD_3 = { "cc-number": "9999888877776666", "cc-exp-month": 1, "cc-exp-year": 2000, }; +const TEST_CREDIT_CARD_WITH_EMPTY_FIELD = { + "cc-name": "", + "cc-number": "1234123412341234", + "cc-exp-month": 1, +}; + const TEST_CREDIT_CARD_WITH_2_DIGITS_YEAR = { "cc-number": "1234123412341234", "cc-exp-month": 1, "cc-exp-year": 12, }; const TEST_CREDIT_CARD_WITH_INVALID_FIELD = { "cc-name": "John Doe", @@ -165,16 +171,22 @@ add_task(async function test_add() { do_check_neq(creditCards[0].guid, undefined); do_check_eq(creditCards[0].version, 1); do_check_neq(creditCards[0].timeCreated, undefined); do_check_eq(creditCards[0].timeLastModified, creditCards[0].timeCreated); do_check_eq(creditCards[0].timeLastUsed, 0); do_check_eq(creditCards[0].timesUsed, 0); + // Empty string should be deleted before saving. + profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_EMPTY_FIELD); + let creditCard = profileStorage.creditCards.data[2]; + do_check_eq(creditCard["cc-exp-month"], TEST_CREDIT_CARD_WITH_EMPTY_FIELD["cc-exp-month"]); + do_check_eq(creditCard["cc-name"], undefined); + Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_FIELD), /"invalidField" is not a valid field\./); }); add_task(async function test_update() { let path = getTempFile(TEST_STORE_FILE_NAME).path; await prepareTestCreditCards(path); @@ -197,16 +209,22 @@ add_task(async function test_update() { await profileStorage.initialize(); let creditCard = profileStorage.creditCards.get(guid); do_check_eq(creditCard["cc-name"], undefined); do_check_neq(creditCard.timeLastModified, timeLastModified); do_check_credit_card_matches(creditCard, TEST_CREDIT_CARD_3); + // Empty string should be deleted while updating. + profileStorage.creditCards.update(profileStorage.creditCards.data[0].guid, TEST_CREDIT_CARD_WITH_EMPTY_FIELD); + creditCard = profileStorage.creditCards.data[0]; + do_check_eq(creditCard["cc-exp-month"], TEST_CREDIT_CARD_WITH_EMPTY_FIELD["cc-exp-month"]); + do_check_eq(creditCard["cc-name"], undefined); + Assert.throws( () => profileStorage.creditCards.update("INVALID_GUID", TEST_CREDIT_CARD_3), /No matching record\./ ); Assert.throws( () => profileStorage.creditCards.update(guid, TEST_CREDIT_CARD_WITH_INVALID_FIELD), /"invalidField" is not a valid field\./