Closed Bug 1370768 Opened 3 years ago Closed 2 years ago

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

Categories

(Toolkit :: Form Manager, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: lchang, Assigned: scottwu)

References

(Blocks 1 open bug)

Details

(Whiteboard: [form autofill:M4] [ETA:8/23])

Attachments

(3 files)

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.
Assignee: nobody → scwwu
Whiteboard: [form autofill:M4] → [form autofill:M4] [ETA:8/10]
Whiteboard: [form autofill:M4] [ETA:8/10] → [form autofill:M4] [ETA:8/23]
Comment on attachment 8900651 [details]
Bug 1370768 - (Part 1) Rename manageAddresses to manageDialog.

https://reviewboard.mozilla.org/r/171620/#review177360
Attachment #8900651 - Flags: review?(lchang) → review+
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 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 on attachment 8901016 [details]
Bug 1370768 - (Part 3) Add browser chrome test for manage credit card dialog.

https://reviewboard.mozilla.org/r/172492/#review177778
Attachment #8901016 - Flags: review?(lchang) → review+
Comment on attachment 8900652 [details]
Bug 1370768 - (Part 2) Add manage credit cards dialog.

https://reviewboard.mozilla.org/r/171618/#review177786
Attachment #8900652 - Flags: review?(lchang) → review+
Thanks Luke! Checking it in.
Keywords: checkin-needed
Pushed by lchang@mozilla.com:
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
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.