From 514f9fcc2988d253e7662a76684b677625fdcc84 Mon Sep 17 00:00:00 2001
From: Matthew Noorenberghe <mozilla@noorenberghe.ca>
Date: Thu, 20 Sep 2018 21:07:20 +0000
Subject: [PATCH] Bug 1463545 - Replace grid layout of <address-option> with a
 new two line design. r=sfoster

Differential Revision: https://phabricator.services.mozilla.com/D5186

--HG--
extra : moz-landing-system : lando
---
 .../res/components/address-option.css         |  65 ++-----
 .../payments/res/components/address-option.js |  97 ++++++++--
 .../payments/res/containers/address-picker.js |  26 ++-
 .../payments/res/containers/payment-dialog.js |  55 ++++--
 .../payments/res/containers/rich-picker.js    |  13 +-
 browser/components/payments/res/debugging.js  |  17 ++
 .../res/mixins/ObservedPropertiesMixin.js     |   6 +-
 .../payments/res/paymentRequest.xhtml         |   3 +
 .../payments/res/unprivileged-fallbacks.js    |   2 +-
 .../payments/test/mochitest/mochitest.ini     |   1 +
 .../test/mochitest/test_address_option.html   | 179 ++++++++++++++++++
 .../test/mochitest/test_address_picker.html   |  14 +-
 .../mochitest/test_payer_address_picker.html  |  29 +--
 .../test/mochitest/test_payment_dialog.html   |   1 +
 ...yment_dialog_required_top_level_items.html |   1 +
 .../test/mochitest/test_rich_select.html      |   3 +
 .../formautofill/content/autofillEditForms.js |   7 +-
 17 files changed, 408 insertions(+), 111 deletions(-)
 create mode 100644 browser/components/payments/test/mochitest/test_address_option.html

diff --git a/browser/components/payments/res/components/address-option.css b/browser/components/payments/res/components/address-option.css
index 4992c71e7d79..9619048ae6ad 100644
--- a/browser/components/payments/res/components/address-option.css
+++ b/browser/components/payments/res/components/address-option.css
@@ -2,67 +2,28 @@
  * 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/. */
 
-address-option {
+address-option.rich-option {
   grid-row-gap: 5px;
-  grid-column-gap: 10px;
-  grid-template-areas:
-    "name          "
-    "street-address";
 }
 
-rich-select[open] > .rich-select-popup-box > address-option {
-  grid-template-areas:
-    "name           name          "
-    "street-address street-address"
-    "email          tel           ";
-}
-
-address-picker.payer-related > rich-select address-option {
-  grid-template-areas:
-    "name name"
-    "tel email";
-}
-
-address-option > .name {
-  grid-area: name;
-}
-
-address-option > .street-address {
-  grid-area: street-address;
-}
-
-address-option > .email {
-  grid-area: email;
-}
-
-address-option > .tel {
-  grid-area: tel;
-}
-
-address-option > .name,
-address-option > .street-address,
-address-option > .email,
-address-option > .tel {
+address-option > .line {
+  overflow: hidden;
+  text-overflow: ellipsis;
   white-space: nowrap;
 }
 
-address-picker.shipping-related address-option > .email,
-address-picker.shipping-related address-option.rich-select-selected-option > .tel {
+address-option > .line:empty {
+  /* Hide the 2nd line in cases where it's empty
+     (e.g. payer field with one or two fields requested) */
   display: none;
 }
 
-/* for payer contact details:
- * display fields selectively based on the contents of the address-fields attribute
- */
-address-picker.payer-related address-option > .name,
-address-picker.payer-related address-option > .street-address,
-address-picker.payer-related address-option > .email,
-address-picker.payer-related address-option > .tel {
-  display: none;
+address-option > .line > span {
+  white-space: nowrap;
 }
 
-address-picker[address-fields~='name'].payer-related address-option  > .name,
-address-picker[address-fields~='email'].payer-related address-option  > .email,
-address-picker[address-fields~='tel'].payer-related address-option  > .tel {
-  display: inline-block;
+address-option > .line > span:empty::before {
+  /* Show the string for missing fields in grey when the field is empty */
+  color: GrayText;
+  content: attr(data-missing-string);
 }
diff --git a/browser/components/payments/res/components/address-option.js b/browser/components/payments/res/components/address-option.js
index 434956bc0e6a..eba1585d1ef1 100644
--- a/browser/components/payments/res/components/address-option.js
+++ b/browser/components/payments/res/components/address-option.js
@@ -2,11 +2,15 @@
  * 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/. */
 
+/* import-globals-from ../../../../../browser/extensions/formautofill/content/autofillEditForms.js*/
 import ObservedPropertiesMixin from "../mixins/ObservedPropertiesMixin.js";
 import RichOption from "./rich-option.js";
 /* import-globals-from ../unprivileged-fallbacks.js */
 
 /**
+ * Up to two-line address display. After bug 1475684 this will also be used for
+ * the single-line <option> substitute too.
+ *
  * <rich-select>
  *  <address-option guid="98hgvnbmytfc"
  *                  address-level1="MI"
@@ -31,6 +35,7 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
       "email",
       "guid",
       "name",
+      "organization",
       "postal-code",
       "street-address",
       "tel",
@@ -38,22 +43,34 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
   }
 
   static get observedAttributes() {
-    return RichOption.observedAttributes.concat(AddressOption.recordAttributes);
+    return RichOption.observedAttributes.concat(AddressOption.recordAttributes,
+                                                "address-fields",
+                                                "break-after-nth-field",
+                                                "data-field-separator");
   }
 
   constructor() {
     super();
 
-    for (let name of ["name", "street-address", "email", "tel"]) {
+    this._line1 = document.createElement("div");
+    this._line1.classList.add("line");
+    this._line2 = document.createElement("div");
+    this._line2.classList.add("line");
+
+    for (let name of AddressOption.recordAttributes) {
       this[`_${name}`] = document.createElement("span");
       this[`_${name}`].classList.add(name);
+      // XXX Bug 1490816: Use appropriate strings
+      let missingValueString = name.replace(/(-|^)([a-z])/g, ($0, $1, $2) => {
+        return $1.replace("-", " ") + $2.toUpperCase();
+      }) + " Missing";
+      this[`_${name}`].dataset.missingString = missingValueString;
     }
   }
 
   connectedCallback() {
-    for (let name of ["name", "street-address", "email", "tel"]) {
-      this.appendChild(this[`_${name}`]);
-    }
+    this.appendChild(this._line1);
+    this.appendChild(this._line2);
     super.connectedCallback();
   }
 
@@ -61,14 +78,72 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
     return PaymentDialogUtils.getAddressLabel(address, addressFields);
   }
 
+  get requiredFields() {
+    if (this.hasAttribute("address-fields")) {
+      let names = this.getAttribute("address-fields").trim().split(/\s+/);
+      if (names.length) {
+        return names;
+      }
+    }
+
+    return [
+      // "address-level1", // TODO: bug 1481481 - not required for some countries e.g. DE
+      "address-level2",
+      "country",
+      "name",
+      "postal-code",
+      "street-address",
+    ];
+  }
+
   render() {
+    // Clear the lines of the fields so we can append only the ones still
+    // visible in the correct order below.
+    this._line1.textContent = "";
+    this._line2.textContent = "";
+
+    // Fill the fields with their text/strings.
     // Fall back to empty strings to prevent 'null' from appearing.
-    this._name.textContent = this.name || "";
-    this["_street-address"].textContent =
-      `${this.streetAddress || ""} ${this.addressLevel2 || ""} ` +
-      `${this.addressLevel1 || ""} ${this.postalCode || ""} ${this.country || ""}`;
-    this._email.textContent = this.email || "";
-    this._tel.textContent = this.tel || "";
+    for (let name of AddressOption.recordAttributes) {
+      let camelCaseName = super.constructor.kebabToCamelCase(name);
+      let fieldEl = this[`_${name}`];
+      fieldEl.textContent = this[camelCaseName] || "";
+    }
+
+    let {fieldsOrder} = PaymentDialogUtils.getFormFormat(this.country);
+    // A subset of the requested fields may be returned if the fields don't apply to the country.
+    let requestedVisibleFields = this.addressFields || "mailing-address";
+    let visibleFields = EditAddress.computeVisibleFields(fieldsOrder, requestedVisibleFields);
+    let visibleFieldCount = 0;
+    let requiredFields = this.requiredFields;
+    // Start by populating line 1
+    let lineEl = this._line1;
+    // Which field number to start line 2 after.
+    let breakAfterNthField = this.breakAfterNthField || 2;
+
+    // Now actually place the fields in the proper place on the lines.
+    for (let field of visibleFields) {
+      let fieldEl = this[`_${field.fieldId}`];
+      if (!fieldEl) {
+        log.warn(`address-option render: '${field.fieldId}' doesn't exist`);
+        continue;
+      }
+
+      if (!fieldEl.textContent && !requiredFields.includes(field.fieldId)) {
+        // The field is empty and we don't need to show "Missing …" so don't append.
+        continue;
+      }
+
+      if (lineEl.children.length > 0) {
+        lineEl.append(this.dataset.fieldSeparator);
+      }
+      lineEl.appendChild(fieldEl);
+
+      // Add a break after this field, if requested.
+      if (++visibleFieldCount == breakAfterNthField) {
+        lineEl = this._line2;
+      }
+    }
   }
 }
 
diff --git a/browser/components/payments/res/containers/address-picker.js b/browser/components/payments/res/containers/address-picker.js
index d8e7bdc9be9a..3665a59bb4cb 100644
--- a/browser/components/payments/res/containers/address-picker.js
+++ b/browser/components/payments/res/containers/address-picker.js
@@ -13,8 +13,16 @@ import paymentRequest from "../paymentRequest.js";
  */
 
 export default class AddressPicker extends RichPicker {
+  static get pickerAttributes() {
+    return [
+      "address-fields",
+      "break-after-nth-field",
+      "data-field-separator",
+    ];
+  }
+
   static get observedAttributes() {
-    return RichPicker.observedAttributes.concat(["address-fields"]);
+    return RichPicker.observedAttributes.concat(AddressPicker.pickerAttributes);
   }
 
   constructor() {
@@ -24,7 +32,7 @@ export default class AddressPicker extends RichPicker {
 
   attributeChangedCallback(name, oldValue, newValue) {
     super.attributeChangedCallback(name, oldValue, newValue);
-    if (name == "address-fields" && oldValue !== newValue) {
+    if (AddressPicker.pickerAttributes.includes(name) && oldValue !== newValue) {
       this.render(this.requestStore.getState());
     }
   }
@@ -100,6 +108,20 @@ export default class AddressPicker extends RichPicker {
         }
       }
 
+      optionEl.dataset.fieldSeparator = this.dataset.fieldSeparator;
+
+      if (this.hasAttribute("address-fields")) {
+        optionEl.setAttribute("address-fields", this.getAttribute("address-fields"));
+      } else {
+        optionEl.removeAttribute("address-fields");
+      }
+
+      if (this.hasAttribute("break-after-nth-field")) {
+        optionEl.setAttribute("break-after-nth-field", this.getAttribute("break-after-nth-field"));
+      } else {
+        optionEl.removeAttribute("break-after-nth-field");
+      }
+
       // fieldNames getter is not used here because it returns a default array with
       // attributes even when "address-fields" observed attribute is null.
       let addressFields = this.getAttribute("address-fields");
diff --git a/browser/components/payments/res/containers/payment-dialog.js b/browser/components/payments/res/containers/payment-dialog.js
index acc56bbf9181..e98757b0fd99 100644
--- a/browser/components/payments/res/containers/payment-dialog.js
+++ b/browser/components/payments/res/containers/payment-dialog.js
@@ -265,6 +265,39 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
     }
   }
 
+  _renderPayerFields(state) {
+    let paymentOptions = state.request.paymentOptions;
+    let payerRequested = this._isPayerRequested(paymentOptions);
+    for (let element of this._payerRelatedEls) {
+      element.hidden = !payerRequested;
+    }
+
+    if (payerRequested) {
+      let fieldNames = new Set(); // default: ["name", "tel", "email"]
+      if (paymentOptions.requestPayerName) {
+        fieldNames.add("name");
+      }
+      if (paymentOptions.requestPayerEmail) {
+        fieldNames.add("email");
+      }
+      if (paymentOptions.requestPayerPhone) {
+        fieldNames.add("tel");
+      }
+      this._payerAddressPicker.setAttribute("address-fields", [...fieldNames].join(" "));
+      // For the payer picker we want to have a line break after the name field (#1)
+      // if all three fields are requested.
+      if (fieldNames.size == 3) {
+        this._payerAddressPicker.setAttribute("break-after-nth-field", 1);
+      } else {
+        this._payerAddressPicker.removeAttribute("break-after-nth-field");
+      }
+    } else {
+      this._payerAddressPicker.removeAttribute("address-fields");
+    }
+    this._payerAddressPicker.dataset.addAddressTitle = this.dataset.payerTitleAdd;
+    this._payerAddressPicker.dataset.editAddressTitle = this.dataset.payerTitleEdit;
+  }
+
   stateChangeCallback(state) {
     super.stateChangeCallback(state);
 
@@ -325,28 +358,8 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
     for (let element of this._shippingRelatedEls) {
       element.hidden = !paymentOptions.requestShipping;
     }
-    let payerRequested = this._isPayerRequested(paymentOptions);
-    for (let element of this._payerRelatedEls) {
-      element.hidden = !payerRequested;
-    }
 
-    if (payerRequested) {
-      let fieldNames = new Set(); // default: ["name", "tel", "email"]
-      if (paymentOptions.requestPayerName) {
-        fieldNames.add("name");
-      }
-      if (paymentOptions.requestPayerEmail) {
-        fieldNames.add("email");
-      }
-      if (paymentOptions.requestPayerPhone) {
-        fieldNames.add("tel");
-      }
-      this._payerAddressPicker.setAttribute("address-fields", [...fieldNames].join(" "));
-    } else {
-      this._payerAddressPicker.removeAttribute("address-fields");
-    }
-    this._payerAddressPicker.dataset.addAddressTitle = this.dataset.payerTitleAdd;
-    this._payerAddressPicker.dataset.editAddressTitle = this.dataset.payerTitleEdit;
+    this._renderPayerFields(state);
 
     // hide the accepted cards list if the merchant didn't specify a preference
     let basicCardMethod = request.paymentMethods
diff --git a/browser/components/payments/res/containers/rich-picker.js b/browser/components/payments/res/containers/rich-picker.js
index 28411d4f0bab..5fe9631540ef 100644
--- a/browser/components/payments/res/containers/rich-picker.js
+++ b/browser/components/payments/res/containers/rich-picker.js
@@ -63,8 +63,15 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
   }
 
   get selectedOption() {
-    return this.dropdown &&
-           this.dropdown.selectedOption;
+    return this.dropdown.selectedOption;
+  }
+
+  get selectedRichOption() {
+    return this.dropdown.selectedRichOption;
+  }
+
+  get requiredFields() {
+    return this.selectedOption ? this.selectedOption.requiredFields || [] : [];
   }
 
   get fieldNames() {
@@ -81,7 +88,7 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
       return [];
     }
 
-    let fieldNames = this.fieldNames;
+    let fieldNames = this.selectedRichOption.requiredFields;
     // Return all field names that are empty or missing from the option.
     return fieldNames.filter(name => !selectedOption.getAttribute(name));
   }
diff --git a/browser/components/payments/res/debugging.js b/browser/components/payments/res/debugging.js
index eff019889f8f..67ebde541af8 100644
--- a/browser/components/payments/res/debugging.js
+++ b/browser/components/payments/res/debugging.js
@@ -220,9 +220,26 @@ let ADDRESSES_1 = {
   "abcde12345": {
     "address-level2": "Mountain View",
     "country": "US",
+    "family-name": "Fields",
+    "given-name": "Mrs.",
     "guid": "abcde12345",
     "name": "Mrs. Fields",
   },
+  "german1": {
+    "additional-name": "Y.",
+    "address-level1": "",
+    "address-level2": "Berlin",
+    "country": "DE",
+    "email": "de@example.com",
+    "family-name": "Mouse",
+    "given-name": "Anon",
+    "guid": "german1",
+    "name": "Anon Y. Mouse",
+    "organization": "Mozilla",
+    "postal-code": "10997",
+    "street-address": "Schlesische Str. 27",
+    "tel": "+49 30 983333002",
+  },
   "missing-country": {
     "address-level1": "ON",
     "address-level2": "Toronto",
diff --git a/browser/components/payments/res/mixins/ObservedPropertiesMixin.js b/browser/components/payments/res/mixins/ObservedPropertiesMixin.js
index 5a0dea95fbe2..034541854e59 100644
--- a/browser/components/payments/res/mixins/ObservedPropertiesMixin.js
+++ b/browser/components/payments/res/mixins/ObservedPropertiesMixin.js
@@ -10,6 +10,10 @@
 
 export default function ObservedPropertiesMixin(superClass) {
   return class ObservedProperties extends superClass {
+    static kebabToCamelCase(name) {
+      return name.replace(/-([a-z])/g, ($0, $1) => $1.toUpperCase());
+    }
+
     constructor() {
       super();
 
@@ -24,7 +28,7 @@ export default function ObservedPropertiesMixin(superClass) {
           continue;
         }
         // Convert attribute names from kebab-case to camelCase properties
-        Object.defineProperty(this, name.replace(/-([a-z])/g, ($0, $1) => $1.toUpperCase()), {
+        Object.defineProperty(this, ObservedProperties.kebabToCamelCase(name), {
           configurable: true,
           get() {
             return this.getAttribute(name);
diff --git a/browser/components/payments/res/paymentRequest.xhtml b/browser/components/payments/res/paymentRequest.xhtml
index 41102546baeb..8d4fc37a36ee 100644
--- a/browser/components/payments/res/paymentRequest.xhtml
+++ b/browser/components/payments/res/paymentRequest.xhtml
@@ -23,6 +23,7 @@
   <!ENTITY deliveryGenericError       "Can’t deliver to this address. Select a different address.">
   <!ENTITY pickupGenericError         "Can’t pick up from this address. Select a different address.">
   <!ENTITY paymentMethodsLabel        "Payment Method">
+  <!ENTITY address.fieldSeparator     ", ">
   <!ENTITY address.addLink.label      "Add">
   <!ENTITY address.editLink.label     "Edit">
   <!ENTITY basicCard.addLink.label    "Add">
@@ -132,6 +133,7 @@
           <address-picker class="shipping-related"
                           data-add-link-label="&address.addLink.label;"
                           data-edit-link-label="&address.editLink.label;"
+                          data-field-separator="&address.fieldSeparator;"
                           data-shipping-address-label="&shippingAddressLabel;"
                           data-delivery-address-label="&deliveryAddressLabel;"
                           data-pickup-address-label="&pickupAddressLabel;"
@@ -155,6 +157,7 @@
                           label="&payerLabel;"
                           data-add-link-label="&payer.addLink.label;"
                           data-edit-link-label="&payer.editLink.label;"
+                          data-field-separator="&address.fieldSeparator;"
                           data-invalid-label="&invalidOption.label;"
                           selected-state-key="selectedPayerAddress"></address-picker>
         </div>
diff --git a/browser/components/payments/res/unprivileged-fallbacks.js b/browser/components/payments/res/unprivileged-fallbacks.js
index bdce0d2dcc6f..6a08e3bb07d3 100644
--- a/browser/components/payments/res/unprivileged-fallbacks.js
+++ b/browser/components/payments/res/unprivileged-fallbacks.js
@@ -81,11 +81,11 @@ var PaymentDialogUtils = {
       "postalCodeLabel": country == "US" ? "zip" : "postalCode",
       "fieldsOrder": [
         {fieldId: "name", newLine: true},
-        {fieldId: "organization", newLine: true},
         {fieldId: "street-address", newLine: true},
         {fieldId: "address-level2"},
         {fieldId: "address-level1"},
         {fieldId: "postal-code"},
+        {fieldId: "organization"},
       ],
       // The following values come from addressReferences.js and should not be changed.
       /* eslint-disable-next-line max-len */
diff --git a/browser/components/payments/test/mochitest/mochitest.ini b/browser/components/payments/test/mochitest/mochitest.ini
index d782e2b8fdf3..9f940851d673 100644
--- a/browser/components/payments/test/mochitest/mochitest.ini
+++ b/browser/components/payments/test/mochitest/mochitest.ini
@@ -13,6 +13,7 @@ skip-if = !e10s
 
 [test_accepted_cards.html]
 [test_address_form.html]
+[test_address_option.html]
 [test_address_picker.html]
 [test_basic_card_form.html]
 [test_basic_card_option.html]
diff --git a/browser/components/payments/test/mochitest/test_address_option.html b/browser/components/payments/test/mochitest/test_address_option.html
new file mode 100644
index 000000000000..92369f36a299
--- /dev/null
+++ b/browser/components/payments/test/mochitest/test_address_option.html
@@ -0,0 +1,179 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Test the address-option component
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test the address-option component</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/AddTask.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <script src="payments_common.js"></script>
+  <script src="../../res/vendor/custom-elements.min.js"></script>
+  <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
+
+  <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
+  <link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+  <p id="display">
+    <option id="option1"
+            data-field-separator=", "
+            address-level1="MI"
+            address-level2="Some City"
+            country="US"
+            email="foo@bar.com"
+            name="John Smith"
+            postal-code="90210"
+            street-address="123 Sesame Street,&#xA;Apt 40"
+            tel="+1 519 555-5555"
+            value="option1"
+            guid="option1"></option>
+    <option id="option2"
+            data-field-separator=", "
+            value="option2"
+            guid="option2"></option>
+
+    <rich-select id="richSelect1"
+                 option-type="address-option"></rich-select>
+  </p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+<script type="module">
+/** Test the address-option component **/
+
+import "../../res/components/address-option.js";
+import "../../res/components/rich-select.js";
+
+let option1 = document.getElementById("option1");
+let option2 = document.getElementById("option2");
+let richSelect1 = document.getElementById("richSelect1");
+
+add_task(async function test_populated_option_rendering() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+
+  is(richOption.name, "John Smith", "Check name getter");
+  is(richOption.streetAddress, "123 Sesame Street,\nApt 40", "Check streetAddress getter");
+  is(richOption.addressLevel2, "Some City", "Check addressLevel2 getter");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith, 123 Sesame Street, Apt 40", "Line 1 text");
+  is(richOption._line2.innerText, "Some City, MI, 90210, US", "Line 2 text");
+
+  // Note that innerText takes visibility into account so that's why it's used over textContent here
+  is(richOption._name.innerText, "John Smith", "name text");
+  is(richOption["_street-address"].innerText, "123 Sesame Street, Apt 40", "street-address text");
+  is(richOption["_address-level2"].innerText, "Some City", "address-level2 text");
+
+  is(richOption._email.parentElement, null,
+     "Check email field isn't in the document for a mailing-address option");
+});
+
+// Same option as the last test but with @break-after-nth-field=1
+add_task(async function test_breakAfterNthField() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  richOption.breakAfterNthField = 1;
+  await asyncElementRendered();
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith", "Line 1 text with breakAfterNthField = 1");
+  is(richOption._line2.innerText, "123 Sesame Street, Apt 40, Some City, MI, 90210, US",
+     "Line 2 text with breakAfterNthField = 1");
+});
+
+add_task(async function test_addressField_mailingAddress() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  richOption.addressFields = "mailing-address";
+  await asyncElementRendered();
+  is(richOption.getAttribute("address-fields"), "mailing-address", "Check @address-fields");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith, 123 Sesame Street, Apt 40", "Line 1 text");
+  is(richOption._line2.innerText, "Some City, MI, 90210, US", "Line 2 text");
+
+  ok(!isHidden(richOption._line2), "Line 2 should be visible when it's used");
+
+  is(richOption._email.parentElement, null,
+     "Check email field isn't in the document for a mailing-address option");
+});
+
+add_task(async function test_addressField_nameEmail() {
+  richSelect1.popupBox.appendChild(option1);
+  richSelect1.value = option1.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  richOption.addressFields = "name email";
+  await asyncElementRendered();
+  is(richOption.getAttribute("address-fields"), "name email", "Check @address-fields");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  ok(!richOption._line1.innerText.trim().endsWith(","), "Line 1 should not end with a comma");
+  ok(!richOption._line2.innerText.trim().endsWith(","), "Line 2 should not end with a comma");
+  is(richOption._line1.innerText, "John Smith, foo@bar.com", "Line 1 text");
+  is(richOption._line2.innerText, "", "Line 2 text");
+
+  ok(isHidden(richOption._line2), "Line 2 should be hidden when it's not used");
+
+  isnot(richOption._email.parentElement, null,
+        "Check email field is in the document for a 'name email' option");
+});
+
+add_task(async function test_missing_fields_option_rendering() {
+  richSelect1.popupBox.appendChild(option2);
+  richSelect1.value = option2.value;
+  await asyncElementRendered();
+
+  let richOption = richSelect1.selectedRichOption;
+  is(richOption.name, null, "Check name getter");
+  is(richOption.streetAddress, null, "Check streetAddress getter");
+  is(richOption.addressLevel2, null, "Check addressLevel2 getter");
+
+  ok(!richOption.innerText.includes("undefined"), "Check for presence of 'undefined'");
+  ok(!richOption.innerText.includes("null"), "Check for presence of 'null'");
+
+  is(richOption._name.innerText, "", "name text");
+  is(window.getComputedStyle(richOption._name, "::before").content, "attr(data-missing-string)",
+     "Check missing field pseudo content");
+  is(richOption._name.getAttribute("data-missing-string"), "Name Missing",
+     "Check @data-missing-string");
+  is(richOption._email.parentElement, null,
+     "Check email field isn't in the document for a mailing-address option");
+});
+
+</script>
+
+</body>
+</html>
diff --git a/browser/components/payments/test/mochitest/test_address_picker.html b/browser/components/payments/test/mochitest/test_address_picker.html
index 23787c3ef54a..51589ac2aec9 100644
--- a/browser/components/payments/test/mochitest/test_address_picker.html
+++ b/browser/components/payments/test/mochitest/test_address_picker.html
@@ -12,6 +12,7 @@ Test the address-picker component
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
@@ -21,6 +22,7 @@ Test the address-picker component
 <body>
   <p id="display">
     <address-picker id="picker1"
+                    data-field-separator=", "
                     selected-state-key="selectedShippingAddress"></address-picker>
   </p>
 <div id="content" style="display: none">
@@ -162,13 +164,13 @@ add_task(async function test_change_selected_address() {
   ok(isHidden(picker1.invalidLabel), "The invalid label should be hidden");
 });
 
-add_task(async function test_streetAddress_combines_street_level2_level1_postalCode_country() {
+add_task(async function test_address_combines_name_street_level2_level1_postalCode_country() {
   let options = picker1.dropdown.popupBox.children;
   let richoption1 = picker1.dropdown.querySelector(".rich-select-selected-option");
-  let streetAddress = richoption1.querySelector(".street-address");
   /* eslint-disable max-len */
-  is(streetAddress.textContent,
-     `${options[1].getAttribute("street-address")} ${options[1].getAttribute("address-level2")} ${options[1].getAttribute("address-level1")} ${options[1].getAttribute("postal-code")} ${options[1].getAttribute("country")}`,
+  is(richoption1.innerText,
+     `${options[1].getAttribute("name")}, ${options[1].getAttribute("street-address")}
+${options[1].getAttribute("address-level2")}, ${options[1].getAttribute("address-level1")}, ${options[1].getAttribute("postal-code")}, ${options[1].getAttribute("country")}`,
      "The address shown should be human readable and include all fields");
   /* eslint-enable max-len */
 
@@ -177,8 +179,8 @@ add_task(async function test_streetAddress_combines_street_level2_level1_postalC
   await asyncElementRendered();
 
   richoption1 = picker1.dropdown.querySelector(".rich-select-selected-option");
-  streetAddress = richoption1.querySelector(".street-address");
-  is(streetAddress.textContent, " Mountain View   US",
+  // "Missing …" text is rendered via a pseudo element content and isn't included in innerText
+  is(richoption1.innerText, "Mrs. Fields, \nMountain View, , US",
      "The address shown should be human readable and include all fields");
 
   picker1.dropdown.popupBox.focus();
diff --git a/browser/components/payments/test/mochitest/test_payer_address_picker.html b/browser/components/payments/test/mochitest/test_payer_address_picker.html
index f2f2c69cd00f..c9964a272193 100644
--- a/browser/components/payments/test/mochitest/test_payer_address_picker.html
+++ b/browser/components/payments/test/mochitest/test_payer_address_picker.html
@@ -12,6 +12,7 @@ Test the paymentOptions address-picker
 
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
@@ -19,7 +20,7 @@ Test the paymentOptions address-picker
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
-  <p id="display">
+  <p id="display" style="height: 100vh; margin: 0;">
     <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"></iframe>
   </p>
 <div id="content" style="display: none">
@@ -126,11 +127,7 @@ add_task(async function setup_once() {
   await SimpleTest.promiseFocus(templateFrame.contentWindow);
 
   let displayEl = document.getElementById("display");
-  // Import the templates from the real shipping dialog to avoid duplication.
-  for (let template of templateFrame.contentDocument.querySelectorAll("template")) {
-    let imported = document.importNode(template, true);
-    displayEl.appendChild(imported);
-  }
+  importDialogDependencies(templateFrame, displayEl);
 
   elDialog = new PaymentDialog();
   displayEl.appendChild(elDialog);
@@ -193,8 +190,7 @@ add_task(async function test_visible_fields() {
     ok(elem, `field ${fieldName} exists`);
     ok(isVisible(elem), `field ${fieldName} is visible`);
   }
-  ok(!isVisible(closedRichOption.querySelector(".street-address")),
-     "street-address is not visible");
+  ok(!closedRichOption.querySelector(".street-address"), "street-address element is not present");
 });
 
 add_task(async function test_selective_fields() {
@@ -207,6 +203,7 @@ add_task(async function test_selective_fields() {
   });
 
   let payerFieldVariations = [
+    {requestPayerName: true, requestPayerEmail: true, requestPayerPhone: true },
     {requestPayerName: true, requestPayerEmail: false, requestPayerPhone: false },
     {requestPayerName: false, requestPayerEmail: true, requestPayerPhone: false },
     {requestPayerName: false, requestPayerEmail: false, requestPayerPhone: true },
@@ -224,12 +221,22 @@ add_task(async function test_selective_fields() {
     let elEmail = closedRichOption.querySelector(".email");
     let elPhone = closedRichOption.querySelector(".tel");
 
-    is(isVisible(elName), payerFields.requestPayerName,
+    is(!!elName && isVisible(elName), payerFields.requestPayerName,
        "name field is correctly toggled");
-    is(isVisible(elEmail), payerFields.requestPayerEmail,
+    is(!!elEmail && isVisible(elEmail), payerFields.requestPayerEmail,
        "email field is correctly toggled");
-    is(isVisible(elPhone), payerFields.requestPayerPhone,
+    is(!!elPhone && isVisible(elPhone), payerFields.requestPayerPhone,
        "tel field is correctly toggled");
+
+    let numPayerFieldsRequested = [...Object.values(payerFields)].filter(val => val).length;
+    is(elPicker.getAttribute("break-after-nth-field"), numPayerFieldsRequested == 3 ? "1" : null,
+       "Check @break-after-nth-field");
+    if (numPayerFieldsRequested == 3) {
+      is(closedRichOption.breakAfterNthField, "1",
+         "Make sure @break-after-nth-field was propagated to <address-option>");
+    } else {
+      is(closedRichOption.breakAfterNthField, null, "Make sure @break-after-nth-field was cleared");
+    }
   }
 });
 
diff --git a/browser/components/payments/test/mochitest/test_payment_dialog.html b/browser/components/payments/test/mochitest/test_payment_dialog.html
index b79436a83c47..6403766a94e1 100644
--- a/browser/components/payments/test/mochitest/test_payment_dialog.html
+++ b/browser/components/payments/test/mochitest/test_payment_dialog.html
@@ -13,6 +13,7 @@ Test the payment-dialog custom element
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/paymentRequest.css"/>
diff --git a/browser/components/payments/test/mochitest/test_payment_dialog_required_top_level_items.html b/browser/components/payments/test/mochitest/test_payment_dialog_required_top_level_items.html
index a9d4a072a076..72a4dc4d0f68 100644
--- a/browser/components/payments/test/mochitest/test_payment_dialog_required_top_level_items.html
+++ b/browser/components/payments/test/mochitest/test_payment_dialog_required_top_level_items.html
@@ -12,6 +12,7 @@ Test the payment-dialog custom element
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/paymentRequest.css"/>
diff --git a/browser/components/payments/test/mochitest/test_rich_select.html b/browser/components/payments/test/mochitest/test_rich_select.html
index 7754ad41822c..dfde1ad34511 100644
--- a/browser/components/payments/test/mochitest/test_rich_select.html
+++ b/browser/components/payments/test/mochitest/test_rich_select.html
@@ -11,6 +11,8 @@ Test the rich-select component
   <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
+  <script src="../../res/unprivileged-fallbacks.js"></script>
+  <script src="autofillEditForms.js"></script>
 
   <link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
@@ -61,6 +63,7 @@ for (let address of Object.values(addresses)) {
   let option = document.createElement("option");
   option.textContent = address.name + " " + address["street-address"];
   option.setAttribute("value", address.guid);
+  option.dataset.fieldSeparator = ", ";
   for (let field of Object.keys(address)) {
     option.setAttribute(field, address[field]);
   }
diff --git a/browser/extensions/formautofill/content/autofillEditForms.js b/browser/extensions/formautofill/content/autofillEditForms.js
index ba75046a140e..37ad5957f409 100644
--- a/browser/extensions/formautofill/content/autofillEditForms.js
+++ b/browser/extensions/formautofill/content/autofillEditForms.js
@@ -160,10 +160,10 @@ class EditAddress extends EditAutofillForm {
    * `mailing-address` is a special attribute token to indicate mailing fields + country.
    *
    * @param {object[]} mailingFieldsOrder - `fieldsOrder` from `getFormFormat`
+   * @param {string} addressFields - white-space-separated string of requested address fields to show
    * @returns {object[]} in the same structure as `mailingFieldsOrder` but including non-mail fields
    */
-  computeVisibleFields(mailingFieldsOrder) {
-    let addressFields = this._elements.form.dataset.addressFields;
+  static computeVisibleFields(mailingFieldsOrder, addressFields) {
     if (addressFields) {
       let requestedFieldClasses = addressFields.trim().split(/\s+/);
       let fieldClasses = [];
@@ -212,7 +212,8 @@ class EditAddress extends EditAutofillForm {
     } = this.getFormFormat(country);
     this._elements.addressLevel1Label.dataset.localization = addressLevel1Label;
     this._elements.postalCodeLabel.dataset.localization = postalCodeLabel;
-    let fieldClasses = this.computeVisibleFields(mailingFieldsOrder);
+    let addressFields = this._elements.form.dataset.addressFields;
+    let fieldClasses = EditAddress.computeVisibleFields(mailingFieldsOrder, addressFields);
     this.arrangeFields(fieldClasses);
     this.updatePostalCodeValidation(postalCodePattern);
   }