1370768 - [Form Autofill] Allow users to view the list of credit card profiles

Status: RESOLVED FIXED

(Toolkit :: Form Manager, enhancement, P3)

Description

[Luke Chang [:lchang] (inactive)]

There will be a list of credit card profiles with the ability to add/delete profiles and open the view/edit dialog for existing profiles.

The dialog to add/view/edit credit card profiles will be implemented separately.

Comment 1

[Scott Wu [:scottwu]]

Attachment: Bug 1370768 - (Part 1) Rename manageAddresses to manageDialog.

Review commit: https://reviewboard.mozilla.org/r/171620/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171620/

Comment 2

[Scott Wu [:scottwu]]

Attachment: Bug 1370768 - (Part 2) Add manage credit cards dialog.

Review commit: https://reviewboard.mozilla.org/r/171618/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171618/

Comment 3

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8900651 [details]
Bug 1370768 - (Part 1) Rename manageAddresses to manageDialog.

https://reviewboard.mozilla.org/r/171620/#review177360

Comment 4

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8900652 [details]
Bug 1370768 - (Part 2) Add manage credit cards dialog.

https://reviewboard.mozilla.org/r/171618/#review177362

::: browser/extensions/formautofill/content/manageCreditCards.xhtml:20
(Diff revision 1)
> +    <legend data-localization="creditCardsListHeader"/>
> +    <select id="credit-cards" size="9" multiple="multiple"/>
> +  </fieldset>
> +  <div id="controls-container">
> +    <button id="remove" disabled="disabled" data-localization="remove"/>
> +    <button id="showCards" data-localization="showCreditCards"/>

nit: id="show-credit-cards"

::: browser/extensions/formautofill/content/manageCreditCards.xhtml:31
(Diff revision 1)
> +    /* global ManageCreditCards */
> +    new ManageCreditCards({
> +      records: document.getElementById("credit-cards"),
> +      controlsContainer: document.getElementById("controls-container"),
> +      remove: document.getElementById("remove"),
> +      showCards: document.getElementById("showCards"),

nit: "show-credit-cards"

::: browser/extensions/formautofill/content/manageDialog.js:134
(Diff revision 1)
> -  },
>  
> -  /**
> -   * Open the edit address dialog to create/edit an address.
> -   *
> -   * @param  {object} address [optional]
> +    // Resume listening to storage change event
> +    Services.obs.addObserver(this, "formautofill-storage-changed");
> +    // For testing only: notify record(s) has been removed
> +    window.dispatchEvent(new CustomEvent("RecordRemoved"));

nit: "RecordsRemoved"

::: browser/extensions/formautofill/content/manageDialog.js:336
(Diff revision 1)
> +   */
> +  async getLabel(creditCard, showCards = false) {
> +    let parts = [];
> +    if (creditCard["cc-number"]) {
> +      parts.push(showCards ? await MasterPassword.decrypt(creditCard["cc-number-encrypted"])
> +                           : `**** ${creditCard["cc-number"].substr(-4)}`);

Is it possible to leverage `_fmtMaskedCreditCardLabel` in ProfileAutoCompleteResult.jsm by moving it to FormAutofillUtils.jsm?

::: browser/extensions/formautofill/content/manageDialog.js:351
(Diff revision 1)
> +    if (event.target == this._elements.showCards) {
> +      this.decryptOptions(this._elements.records.options);
> +    }

Should we hide/disable the button or change the label to "Hide Credit Cards" once it's clicked? Login Manager does change the label. (It's actually a UX question.)

Comment 5

[Scott Wu [:scottwu]]

Comment on attachment 8900652 [details]
Bug 1370768 - (Part 2) Add manage credit cards dialog.

https://reviewboard.mozilla.org/r/171618/#review177362

Thanks Luke. Updating the patch and will include browser chrome test.

> Should we hide/disable the button or change the label to "Hide Credit Cards" once it's clicked? Login Manager does change the label. (It's actually a UX question.)

Yeah so we have multiple options: hide the button, change the label, or disable the button. Will ask UX next week if this needs improvement, but for now we can stick with the spec.

Comment 6

[Scott Wu [:scottwu]]

Attachment: Bug 1370768 - (Part 3) Add browser chrome test for manage credit card dialog.

Review commit: https://reviewboard.mozilla.org/r/172492/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/172492/

Comment 7

[Scott Wu [:scottwu]]

Comment on attachment 8900652 [details]
Bug 1370768 - (Part 2) Add manage credit cards dialog.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171618/diff/1-2/

Comment 8

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8901016 [details]
Bug 1370768 - (Part 3) Add browser chrome test for manage credit card dialog.

https://reviewboard.mozilla.org/r/172492/#review177778

Comment 9

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8900652 [details]
Bug 1370768 - (Part 2) Add manage credit cards dialog.

https://reviewboard.mozilla.org/r/171618/#review177786

Comment 10

[Scott Wu [:scottwu]]

Thanks Luke! Checking it in.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52168d31f429
(Part 1) Rename manageAddresses to manageDialog. r=lchang
https://hg.mozilla.org/integration/autoland/rev/8b7f29c9028d
(Part 2) Add manage credit cards dialog. r=lchang
https://hg.mozilla.org/integration/autoland/rev/5ec1cb93ebc8
(Part 3) Add browser chrome test for manage credit card dialog. r=lchang

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/52168d31f429
https://hg.mozilla.org/mozilla-central/rev/8b7f29c9028d
https://hg.mozilla.org/mozilla-central/rev/5ec1cb93ebc8