Bug 1428414 - Removed cleared record attributes on card and address options. r=jaws
If a property gets cleared in autofill storage, that property is not defined on the object when retrieved which means it won't get updated.
Previously I thought that known fields were always returned and had a default value.
MozReview-Commit-ID: KnBrpFdOk8P
--- a/toolkit/components/payments/res/components/address-option.js
+++ b/toolkit/components/payments/res/components/address-option.js
@@ -16,28 +16,32 @@
* </rich-select>
*
* Attribute names follow FormAutofillStorage.jsm.
*/
/* global ObservedPropertiesMixin, RichOption */
class AddressOption extends ObservedPropertiesMixin(RichOption) {
- static get observedAttributes() {
- return RichOption.observedAttributes.concat([
+ static get recordAttributes() {
+ return [
"address-level1",
"address-level2",
"country",
"email",
"guid",
"name",
"postal-code",
"street-address",
"tel",
- ]);
+ ];
+ }
+
+ static get observedAttributes() {
+ return RichOption.observedAttributes.concat(AddressOption.recordAttributes);
}
constructor() {
super();
for (let name of ["name", "street-address", "email", "tel"]) {
this[`_${name}`] = document.createElement("span");
this[`_${name}`].classList.add(name);
--- a/toolkit/components/payments/res/components/basic-card-option.js
+++ b/toolkit/components/payments/res/components/basic-card-option.js
@@ -6,24 +6,28 @@
* <rich-select>
* <basic-card-option></basic-card-option>
* </rich-select>
*/
/* global ObservedPropertiesMixin, RichOption */
class BasicCardOption extends ObservedPropertiesMixin(RichOption) {
- static get observedAttributes() {
- return RichOption.observedAttributes.concat([
+ static get recordAttributes() {
+ return [
"cc-exp",
"cc-name",
"cc-number",
"guid",
"type", // XXX Bug 1429181.
- ]);
+ ];
+ }
+
+ static get observedAttributes() {
+ return RichOption.observedAttributes.concat(BasicCardOption.recordAttributes);
}
constructor() {
super();
for (let name of ["cc-name", "cc-number", "cc-exp", "type"]) {
this[`_${name}`] = document.createElement("span");
this[`_${name}`].classList.add(name);
--- a/toolkit/components/payments/res/containers/address-picker.js
+++ b/toolkit/components/payments/res/containers/address-picker.js
@@ -1,13 +1,14 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-/* global PaymentStateSubscriberMixin */
+/* import-globals-from ../mixins/PaymentStateSubscriberMixin.js */
+/* import-globals-from ../components/address-option.js */
"use strict";
/**
* <address-picker></address-picker>
* Container around <rich-select> (eventually providing add/edit links) with
* <address-option> listening to savedAddresses.
*/
@@ -33,40 +34,38 @@ class AddressPicker extends PaymentState
this.render(this.requestStore.getState());
}
}
/**
* De-dupe and filter addresses for the given set of fields that will be visible
*
* @param {object} addresses
- * @param {array?} fieldNames - optional list of field names that be used when de-duping entries
+ * @param {array?} fieldNames - optional list of field names that be used when
+ * de-duping and excluding entries
* @returns {object} filtered copy of given addresses
*/
filterAddresses(addresses, fieldNames = [
"address-level1",
"address-level2",
"country",
"name",
"postal-code",
"street-address",
]) {
let uniques = new Set();
let result = {};
for (let [guid, address] of Object.entries(addresses)) {
let addressCopy = {};
- let isMatch;
- // exclude addresses that are missing any of the requested fields
+ let isMatch = false;
+ // exclude addresses that are missing all of the requested fields
for (let name of fieldNames) {
if (address[name]) {
isMatch = true;
addressCopy[name] = address[name];
- } else {
- isMatch = false;
- break;
}
}
if (isMatch) {
let key = JSON.stringify(addressCopy);
// exclude duplicated addresses
if (!uniques.has(key)) {
uniques.add(key);
result[guid] = address;
@@ -90,19 +89,25 @@ class AddressPicker extends PaymentState
for (let [guid, address] of Object.entries(filteredAddresses)) {
let optionEl = this.dropdown.getOptionByValue(guid);
if (!optionEl) {
optionEl = document.createElement("address-option");
optionEl.value = guid;
}
- for (let [key, val] of Object.entries(address)) {
- optionEl.setAttribute(key, val);
+ for (let key of AddressOption.recordAttributes) {
+ let val = address[key];
+ if (val) {
+ optionEl.setAttribute(key, val);
+ } else {
+ optionEl.removeAttribute(key);
+ }
}
+
desiredOptions.push(optionEl);
}
let el = null;
while ((el = this.dropdown.popupBox.querySelector(":scope > address-option"))) {
el.remove();
}
for (let option of desiredOptions) {
--- a/toolkit/components/payments/res/containers/payment-method-picker.js
+++ b/toolkit/components/payments/res/containers/payment-method-picker.js
@@ -1,13 +1,14 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-/* global PaymentStateSubscriberMixin */
+/* import-globals-from ../mixins/PaymentStateSubscriberMixin.js */
+/* import-globals-from ../components/basic-card-option.js */
"use strict";
/**
* <payment-method-picker></payment-method-picker>
* Container around add/edit links and <rich-select> with
* <basic-card-option> listening to savedBasicCards.
*/
@@ -46,18 +47,23 @@ class PaymentMethodPicker extends Paymen
let {savedBasicCards} = state;
let desiredOptions = [];
for (let [guid, basicCard] of Object.entries(savedBasicCards)) {
let optionEl = this.dropdown.getOptionByValue(guid);
if (!optionEl) {
optionEl = document.createElement("basic-card-option");
optionEl.value = guid;
}
- for (let [key, val] of Object.entries(basicCard)) {
- optionEl.setAttribute(key, val);
+ for (let key of BasicCardOption.recordAttributes) {
+ let val = basicCard[key];
+ if (val) {
+ optionEl.setAttribute(key, val);
+ } else {
+ optionEl.removeAttribute(key);
+ }
}
desiredOptions.push(optionEl);
}
let el = null;
while ((el = this.dropdown.popupBox.querySelector(":scope > basic-card-option"))) {
el.remove();
}
for (let option of desiredOptions) {
--- a/toolkit/components/payments/test/mochitest/test_address_picker.html
+++ b/toolkit/components/payments/test/mochitest/test_address_picker.html
@@ -80,17 +80,17 @@ add_task(async function test_initialSet(
});
add_task(async function test_update() {
picker1.requestStore.setState({
savedAddresses: {
"48bnds6854t": {
// Same GUID, different values to trigger an update
"address-level1": "MI-edit",
- "address-level2": "Some City-edit",
+ // address-level2 was cleared which means it's not returned
"country": "CA",
"guid": "48bnds6854t",
"name": "Mr. Foo-edit",
"postal-code": "90210-1234",
"street-address": "new-edit",
"tel": "+1 650 555-5555",
},
"68gjdh354j": {
@@ -104,17 +104,17 @@ add_task(async function test_update() {
"tel": "+1 650 555-5555",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 2, "Check dropdown still has both addresses");
ok(options[0].textContent.includes("MI-edit"), "Check updated first address-level1");
- ok(options[0].textContent.includes("Some City-edit"), "Check updated first address-level2");
+ ok(!options[0].textContent.includes("Some City"), "Check removed first address-level2");
ok(options[0].textContent.includes("new-edit"), "Check updated first address");
ok(options[1].textContent.includes("P.O. Box 123"), "Check second address is the same");
});
add_task(async function test_change_selected_address() {
let options = picker1.dropdown.popupBox.children;
let selectedOption = picker1.dropdown.selectedOption;
--- a/toolkit/components/payments/test/mochitest/test_payer_address_picker.html
+++ b/toolkit/components/payments/test/mochitest/test_payer_address_picker.html
@@ -258,17 +258,17 @@ add_task(async function test_filtered_op
selectedPayerAddress: "a9e830667189",
});
await asyncElementRendered();
let visibleOptions = getVisiblePickerOptions(elPicker);
let visibleOption = visibleOptions[0];
- is(elPicker.dropdown.popupBox.children.length, 3, "Check dropdown has 3 addresses");
+ is(elPicker.dropdown.popupBox.children.length, 4, "Check dropdown has 3 addresses");
is(visibleOptions.length, 1, "One option should be visible");
is(visibleOption.getAttribute("guid"), "a9e830667189", "expected option is visible");
for (let fieldName of ["name", "email"]) {
let elem = visibleOption.querySelector(`.${fieldName}`);
ok(elem, `field ${fieldName} exists`);
ok(isVisible(elem), `field ${fieldName} is visible`);
}
@@ -287,22 +287,31 @@ add_task(async function test_filtered_op
is(elPicker.dropdown.popupBox.children.length, 4, "Check dropdown has 4 addresses");
is(visibleOptions.length, 1, "One option should be visible");
});
add_task(async function test_no_matches() {
await setup();
let requestStore = elPicker.requestStore;
setPaymentOptions(requestStore, {
- requestPayerEmail: true,
requestPayerPhone: true,
});
requestStore.setState({
- savedAddresses: DUPED_ADDRESSES,
+ savedAddresses: {
+ "2b4dce0fbc1f": {
+ "email": "rita@foo.com",
+ "guid": "2b4dce0fbc1f",
+ "name": "Rita Foo",
+ },
+ "46b2635a5b26": {
+ "guid": "46b2635a5b26",
+ "name": "Rita Foo",
+ },
+ },
selectedPayerAddress: "a9e830667189",
});
await asyncElementRendered();
let visibleOptions = getVisiblePickerOptions(elPicker);
is(elPicker.dropdown.popupBox.children.length, 0, "Check dropdown is empty");
is(visibleOptions.length, 0, "No options should be visible");
--- a/toolkit/components/payments/test/mochitest/test_payment_method_picker.html
+++ b/toolkit/components/payments/test/mochitest/test_payment_method_picker.html
@@ -78,34 +78,34 @@ add_task(async function test_initialSet(
add_task(async function test_update() {
picker1.requestStore.setState({
savedBasicCards: {
"48bnds6854t": {
// Same GUID, different values to trigger an update
"cc-exp": "2017-09",
"cc-exp-month": 9,
"cc-exp-year": 2017,
- "cc-name": "John Edit Doe",
+ // cc-name was cleared which means it's not returned
"cc-number": "************9876",
"guid": "48bnds6854t",
},
"68gjdh354j": {
"cc-exp": "2017-08",
"cc-exp-month": 8,
"cc-exp-year": 2017,
"cc-name": "J Smith",
"cc-number": "***********1234",
"guid": "68gjdh354j",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 2, "Check dropdown still has both cards");
- ok(options[0].textContent.includes("John Edit Doe"), "Check updated first cc-name");
+ ok(!options[0].textContent.includes("John Doe"), "Check cleared first cc-name");
ok(options[0].textContent.includes("9876"), "Check updated first cc-number");
ok(options[0].textContent.includes("09"), "Check updated first exp-month");
ok(options[1].textContent.includes("J Smith"), "Check second card is the same");
});
add_task(async function test_change_selected_card() {
let options = picker1.dropdown.popupBox.children;