Bug 1854056 - removing affix from ProfileAutoCompleteResult r=mtigley,credential-management-reviewers

Depends on D191210 Differential Revision: https://phabricator.services.mozilla.com/D191355
2023-10-30 18:38:10 +00:00 · 2023-10-30 18:38:10 +00:00 · 2751c582e0
--- a/browser/extensions/formautofill/content/customElements.js
+++ b/browser/extensions/formautofill/content/customElements.js
@ -83,7 +83,6 @@
      return `
        <div xmlns="http://www.w3.org/1999/xhtml" class="autofill-item-box">
          <div class="profile-label-col profile-item-col">
-            <span class="profile-label-affix"></span>
            <span class="profile-label"></span>
          </div>
          <div class="profile-comment-col profile-item-col">
@ -103,7 +102,6 @@
      this.appendChild(this.constructor.fragment);

      this._itemBox = this.querySelector(".autofill-item-box");
-      this._labelAffix = this.querySelector(".profile-label-affix");
      this._label = this.querySelector(".profile-label");
      this._comment = this.querySelector(".profile-comment");

@ -139,11 +137,10 @@
        `url(${this.getAttribute("ac-image")})`
      );

-      let { primaryAffix, primary, secondary, ariaLabel } = JSON.parse(
+      let { primary, secondary, ariaLabel } = JSON.parse(
        this.getAttribute("ac-value")
      );

-      this._labelAffix.textContent = primaryAffix;
      this._label.textContent = primary;
      this._comment.textContent = secondary;
      if (ariaLabel) {
--- a/browser/extensions/formautofill/skin/shared/autocomplete-item-shared.css
+++ b/browser/extensions/formautofill/skin/shared/autocomplete-item-shared.css
@ -31,7 +31,6 @@ xul|richlistitem[originaltype="autofill-insecureWarning"] {
  --warning-background-color: rgba(248, 232, 28, .2);

  --default-font-size: 12;
-  --label-affix-font-size: 10;
  --label-font-size: 12;
  --comment-font-size: 10;
  --warning-font-size: 10;
@ -106,11 +105,6 @@ xul|richlistitem[originaltype="autofill-insecureWarning"] {
  unicode-bidi: plaintext;
 }

-.autofill-item-box > .profile-label-col > .profile-label-affix {
-  font-weight: lighter;
-  font-size: calc(var(--label-affix-font-size) / var(--default-font-size) * 1em);
-}
-
 .autofill-item-box > .profile-comment-col {
  margin-inline-start: var(--col-spacer);
  text-align: end;
--- a/browser/extensions/formautofill/test/mochitest/creditCard/test_basic_creditcard_autocomplete_form.html
+++ b/browser/extensions/formautofill/test/mochitest/creditCard/test_basic_creditcard_autocomplete_form.html
@ -86,8 +86,8 @@ add_task(async function all_saved_fields_less_than_threshold() {
  synthesizeKey("KEY_ArrowDown");
  checkMenuEntries([reducedMockRecord].map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-name"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `Visa ${cc["cc-name"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `Visa ${cc["cc-name"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));

@ -102,10 +102,9 @@ add_task(async function check_menu_when_both_existed() {
  await expectPopup();
  synthesizeKey("KEY_ArrowDown");
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
-    primaryAffix: cc.ccNumberFmt.affix,
-    primary: cc.ccNumberFmt.label,
+    primary: cc.ccNumberFmt,
    secondary: cc["cc-name"],
-    ariaLabel: `${getCCTypeName(cc)} ${cc.ccNumberFmt.affix} ${cc.ccNumberFmt.label} ${cc["cc-name"]}`,
+    ariaLabel: `${getCCTypeName(cc)} ${cc.ccNumberFmt.replaceAll("*", "")} ${cc["cc-name"]}`,
    image: expected.image,
  })));

@ -114,8 +113,8 @@ add_task(async function check_menu_when_both_existed() {
  synthesizeKey("KEY_ArrowDown");
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-name"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));

@ -124,8 +123,8 @@ add_task(async function check_menu_when_both_existed() {
  synthesizeKey("KEY_ArrowDown");
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-exp-year"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-exp-year"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-exp-year"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));

@ -134,8 +133,8 @@ add_task(async function check_menu_when_both_existed() {
  synthesizeKey("KEY_ArrowDown");
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-exp-month"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-exp-month"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-exp-month"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));

@ -187,8 +186,8 @@ add_task(async function check_fields_after_form_autofill() {
  await expectPopup();
  checkMenuEntries(MOCK_STORAGE.slice(1).map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-exp-year"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-exp-year"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-exp-year"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));

@ -221,8 +220,8 @@ add_task(async function check_cc_popup_on_field_blank() {
  await expectPopup();
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-name"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));
 });
@ -241,8 +240,8 @@ add_task(async function check_form_autofill_resume() {
  await expectPopup();
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-name"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));
 });
--- a/browser/extensions/formautofill/test/mochitest/creditCard/test_creditcard_autocomplete_off.html
+++ b/browser/extensions/formautofill/test/mochitest/creditCard/test_creditcard_autocomplete_off.html
@ -73,10 +73,9 @@ add_task(async function check_menu_when_both_with_autocomplete_off() {
  synthesizeKey("KEY_ArrowDown");
  await expectPopup();
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
-    primaryAffix: cc.ccNumberFmt.affix,
-    primary: cc.ccNumberFmt.label,
+    primary: cc.ccNumberFmt,
    secondary: cc["cc-name"],
-    ariaLabel: `${getCCTypeName(cc)} ${cc.ccNumberFmt.affix} ${cc.ccNumberFmt.label} ${cc["cc-name"]}`,
+    ariaLabel: `${getCCTypeName(cc)} ${cc.ccNumberFmt.replaceAll("*", "")} ${cc["cc-name"]}`,
    image: expected.image,
  })));

@ -85,8 +84,8 @@ add_task(async function check_menu_when_both_with_autocomplete_off() {
  await expectPopup();
  checkMenuEntries(MOCK_STORAGE.map(patchRecordCCNumber).map(({ cc, expected }) => JSON.stringify({
    primary: cc["cc-name"],
-    secondary: cc.ccNumberFmt.affix + cc.ccNumberFmt.label,
-    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt.affix}${cc.ccNumberFmt.label}`,
+    secondary: cc.ccNumberFmt,
+    ariaLabel: `${getCCTypeName(cc)} ${cc["cc-name"]} ${cc.ccNumberFmt}`,
    image: expected.image,
  })));
 });
--- a/browser/extensions/formautofill/test/mochitest/formautofill_common.js
+++ b/browser/extensions/formautofill/test/mochitest/formautofill_common.js
@ -371,11 +371,7 @@ async function waitForOSKeyStoreLogin(login = false) {
 }

 function patchRecordCCNumber(record) {
-  const number = record.cc["cc-number"];
-  const ccNumberFmt = {
-    affix: "****",
-    label: number.substr(-4),
-  };
+  const ccNumberFmt = "****" + record.cc["cc-number"].substr(-4);

  return {
    cc: Object.assign({}, record.cc, { ccNumberFmt }),
--- a/browser/extensions/formautofill/test/unit/test_profileAutocompleteResult.js
+++ b/browser/extensions/formautofill/test/unit/test_profileAutocompleteResult.js
@ -328,10 +328,9 @@ let creditCardTestCases = [
          style: "autofill-profile",
          comment: JSON.stringify(matchingProfiles[0]),
          label: JSON.stringify({
-            primaryAffix: "****",
-            primary: "6785",
+            primary: "****6785",
            secondary: "Timothy Berners-Lee",
-            ariaLabel: "Visa **** 6785 Timothy Berners-Lee",
+            ariaLabel: "Visa 6785 Timothy Berners-Lee",
            image: "chrome://formautofill/content/third-party/cc-logo-visa.svg",
          }),
          image: "chrome://formautofill/content/third-party/cc-logo-visa.svg",
@ -341,10 +340,9 @@ let creditCardTestCases = [
          style: "autofill-profile",
          comment: JSON.stringify(matchingProfiles[1]),
          label: JSON.stringify({
-            primaryAffix: "****",
-            primary: "1234",
+            primary: "****1234",
            secondary: "John Doe",
-            ariaLabel: "American Express **** 1234 John Doe",
+            ariaLabel: "American Express 1234 John Doe",
            image: "chrome://formautofill/content/third-party/cc-logo-amex.png",
          }),
          image: "chrome://formautofill/content/third-party/cc-logo-amex.png",
@ -354,10 +352,9 @@ let creditCardTestCases = [
          style: "autofill-profile",
          comment: JSON.stringify(matchingProfiles[2]),
          label: JSON.stringify({
-            primaryAffix: "****",
-            primary: "5678",
+            primary: "****5678",
            secondary: "",
-            ariaLabel: "**** 5678",
+            ariaLabel: "5678",
            image: "chrome://formautofill/content/icon-credit-card-generic.svg",
          }),
          image: "chrome://formautofill/content/icon-credit-card-generic.svg",
--- a/toolkit/components/formautofill/ProfileAutoCompleteResult.sys.mjs
+++ b/toolkit/components/formautofill/ProfileAutoCompleteResult.sys.mjs
@ -363,10 +363,7 @@ export class CreditCardResult extends ProfileAutoCompleteResult {

      if (matching) {
        if (currentFieldName == "cc-number") {
-          let { affix, label } = lazy.CreditCard.formatMaskedNumber(
-            profile[currentFieldName]
-          );
-          return affix + label;
+          return lazy.CreditCard.formatMaskedNumber(profile[currentFieldName]);
        }
        return profile[currentFieldName];
      }
@ -401,13 +398,10 @@ export class CreditCardResult extends ProfileAutoCompleteResult {
        return !!profile[focusedFieldName];
      })
      .map(profile => {
-        let primaryAffix;
        let primary = profile[focusedFieldName];

        if (focusedFieldName == "cc-number") {
-          let { affix, label } = lazy.CreditCard.formatMaskedNumber(primary);
-          primaryAffix = affix;
-          primary = label;
+          primary = lazy.CreditCard.formatMaskedNumber(primary);
        }
        const secondary = this._getSecondaryLabel(
          focusedFieldName,
@ -423,11 +417,10 @@ export class CreditCardResult extends ProfileAutoCompleteResult {
        const ccTypeName = ccTypeL10nId
          ? lazy.l10n.formatValueSync(ccTypeL10nId)
          : ccType ?? ""; // Unknown card type
-        const ariaLabel = [ccTypeName, primaryAffix, primary, secondary]
+        const ariaLabel = [ccTypeName, primary.toString().replaceAll("*", ""), secondary]
          .filter(chunk => !!chunk) // Exclude empty chunks.
          .join(" ");
        return {
-          primaryAffix,
          primary,
          secondary,
          ariaLabel,
--- a/toolkit/modules/CreditCard.sys.mjs
+++ b/toolkit/modules/CreditCard.sys.mjs
@ -454,10 +454,15 @@ export class CreditCard {
  }

  static formatMaskedNumber(maskedNumber) {
-    return {
-      affix: "****",
-      label: maskedNumber.replace(/^\**/, ""),
-    };
+    return "*".repeat(4) + maskedNumber.substr(-4);
+  }
+
+  static getMaskedNumber(number) {
+    return "*".repeat(4) + " " + number.substr(-4);
+  }
+
+  static getLongMaskedNumber(number) {
+    return "*".repeat(number.length - 4) + number.substr(-4);
  }

  static getCreditCardLogo(network) {
@ -487,14 +492,6 @@ export class CreditCard {
    }
  }

-  static getMaskedNumber(number) {
-    return "*".repeat(4) + " " + number.substr(-4);
-  }
-
-  static getLongMaskedNumber(number) {
-    return "*".repeat(number.length - 4) + number.substr(-4);
-  }
-
  /*
   * Validates the number according to the Luhn algorithm. This
   * method does not throw an exception if the number is invalid.
--- a/toolkit/modules/tests/xpcshell/test_CreditCard.js
+++ b/toolkit/modules/tests/xpcshell/test_CreditCard.js
@ -122,20 +122,15 @@ add_task(function isValidNumber() {
 });

 add_task(function test_formatMaskedNumber() {
-  function testFormat(number) {
-    let format = CreditCard.formatMaskedNumber(number);
-    Assert.equal(format.affix, "****", "Affix should always be four asterisks");
-    Assert.equal(
-      format.label,
-      number.substr(-4),
-      "The label should always be the last four digits of the card number"
-    );
+  function assertMaskedNumber(input, expected) {
+    const actual = CreditCard.formatMaskedNumber(input);
+    Assert.equal(actual, expected);
  }
-  testFormat("************0000");
-  testFormat("************1045");
-  testFormat("***********6806");
-  testFormat("**********0495");
-  testFormat("**********8250");
+  assertMaskedNumber("************0000", "****0000");
+  assertMaskedNumber("************1045", "****1045");
+  assertMaskedNumber("***********6806", "****6806");
+  assertMaskedNumber("**********0495", "****0495");
+  assertMaskedNumber("**********8250", "****8250");
 });

 add_task(function test_maskNumber() {