[Form Autofill] Implement the credit-card storage

RESOLVED FIXED in Firefox 55

Status

()

P3
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: lchang, Assigned: lchang)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [form autofill:M3])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Let's extend the ProfileStorage.jsm to store credit card information.
(Assignee)

Updated

2 years ago
Assignee: nobody → lchang
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8863062 - Flags: review?(schung)
(Assignee)

Updated

2 years ago
Attachment #8863083 - Flags: review?(schung)
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139552

::: browser/extensions/formautofill/FormAutofillParent.jsm:157
(Diff revision 1)
>    _getStatus() {
>      if (!Services.prefs.getBoolPref(ENABLED_PREF)) {
>        return false;
>      }
>  
> -    return this._profileStore.getAll().length > 0;
> +    return this._profileStore.profiles.getAll().length > 0;

I thought you were changing this to `.addresses`?
(Assignee)

Comment 5

2 years ago
mozreview-review-reply
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139552

> I thought you were changing this to `.addresses`?

Yeah. I'm thinking to put it in part 3 because Juwei hasn't made the final decision on this naming. Besides, some APIs (e.g. `GetProfiles`) will need to be changed as well.
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139556

I didn't finish reviewing this yet but here are some comments

::: commit-message-2d595:1
(Diff revision 1)
> +Bug 1359978 - (Part 1) Refactor ProfileStorage.jsm. r=MattN

Please include more details in this (first line) of the commit message to explain what is changing.

::: browser/extensions/formautofill/ProfileStorage.jsm:49
(Diff revision 1)
> +/* exported ProfileStorage */
>  this.EXPORTED_SYMBOLS = ["ProfileStorage"];

I thought this was automatic using EXPORTED_SYMBOLS?

::: browser/extensions/formautofill/ProfileStorage.jsm:68
(Diff revision 1)
>  this.log = null;
>  FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);

Rather than modifying all the log calls to include the collection name, how about moving this to inside the base class constructor and including the collection name in the 2nd argument? Then we would log with `this.log.debug` instead of `log.debug`.

::: browser/extensions/formautofill/ProfileStorage.jsm:95
(Diff revision 1)
> -  guid: "test-guid-1",
> -  organization: "Sesame Street",
> -  "street-address": "123 Sesame Street.",
> -  tel: "1-345-345-3456",
> -}, {
> -  guid: "test-guid-2",
> +  "timeLastUsed",
> +  "timeLastModified",
> +  "timesUsed",
> +];
> +
> +class DataHandler {

"AutofillRecord" or "AutofillProfile" would be clearer IMO

::: browser/extensions/formautofill/ProfileStorage.jsm:96
(Diff revision 1)
> -  organization: "Sesame Street",
> -  "street-address": "123 Sesame Street.",
> -  tel: "1-345-345-3456",
> -}, {
> -  guid: "test-guid-2",
> -  organization: "Mozilla",
> +  "timeLastModified",
> +  "timesUsed",
> +];
> +
> +class DataHandler {
> +  constructor(store, dataId, validFields) {

Nit: dataId could maybe be called `collectionName` or just `collection` but that's longer…

"Id" made me think it was something more like a GUID. Thoughts?
(Assignee)

Comment 7

2 years ago
mozreview-review-reply
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139556

> Nit: dataId could maybe be called `collectionName` or just `collection` but that's longer…
> 
> "Id" made me think it was something more like a GUID. Thoughts?

Agree. That would be better.

Comment 8

2 years ago
mozreview-review
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139574

I only have some questions but it looks fine basically. I would like to know how it integrate with Bug 1361965

::: commit-message-2d595:1
(Diff revision 1)
> +Bug 1359978 - (Part 1) Refactor ProfileStorage.jsm. r=MattN

Maybe creating more commits for differnt purposes?

::: browser/extensions/formautofill/ProfileStorage.jsm:96
(Diff revision 1)
> -  organization: "Sesame Street",
> -  "street-address": "123 Sesame Street.",
> -  tel: "1-345-345-3456",
> -}, {
> -  guid: "test-guid-2",
> -  organization: "Mozilla",
> +  "timeLastModified",
> +  "timesUsed",
> +];
> +
> +class DataHandler {
> +  constructor(store, dataId, validFields) {

+1 that there could be other better naming than ID

::: browser/extensions/formautofill/ProfileStorage.jsm:120
(Diff revision 1)
>  
> -    let profileToSave = this._clone(profile);
> -    this._normalizeProfile(profileToSave);
> +    let dataToSave = this._clone(data);
> +    this._normalizeData(dataToSave);
>  
> -    profileToSave.guid = gUUIDGenerator.generateUUID().toString()
> +    let guid;
> +    while (!guid || this._findByGUID(guid)) {

Is that necessary to check the duplicate guid, since you already applied `gUUIDGenerator` here?

::: browser/extensions/formautofill/ProfileStorage.jsm:285
(Diff revision 1)
>    _findByGUID(guid) {
> -    return this._store.data.profiles.find(profile => profile.guid == guid);
> -  },
> +    let found = this._findIndexByGUID(guid);
> +    return found < 0 ? undefined : this._store.data[this._dataId][found];
> +  }
> +
> +  _findIndexByGUID(guid) {

Sorry but I'm not sure why we need this `_findIndexByGUID` here...

::: browser/extensions/formautofill/ProfileStorage.jsm:391
(Diff revision 1)
> +    this.INTERNAL_FIELDS = INTERNAL_FIELDS;
> +  }
>  
> -    for (let key in profile) {
> -      if (!VALID_FIELDS.includes(key)) {
> -        throw new Error(`"${key}" is not a valid field.`);
> +  get profiles() {
> +    if (!this._profiles) {
> +      this._store.ensureDataReady();

Is it sufficient to have `ensureDataReady` only here? I thought it would be called once while initializing profile here.
Attachment #8863062 - Flags: review?(schung)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 11

2 years ago
mozreview-review-reply
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139556

> Rather than modifying all the log calls to include the collection name, how about moving this to inside the base class constructor and including the collection name in the 2nd argument? Then we would log with `this.log.debug` instead of `log.debug`.

Good point. Thanks.
(Assignee)

Comment 12

2 years ago
mozreview-review-reply
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review139574

> Is that necessary to check the duplicate guid, since you already applied `gUUIDGenerator` here?

I'm not sure, either. I'm just afraid the conflicts because we truncate the UUID into 12 characters only.

> Sorry but I'm not sure why we need this `_findIndexByGUID` here...

I noticed that sync team implemented this function in their WIP patch so I prepared this for them.

> Is it sufficient to have `ensureDataReady` only here? I thought it would be called once while initializing profile here.

I think so as everyone who wants to manipulate the records needs to retrieve a `profiles` here. In other words, data should be always ready once the `profiles` object is initialized.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Depends on: 1362934
(Assignee)

Updated

2 years ago
Attachment #8863062 - Flags: review?(schung)
Attachment #8863062 - Flags: review?(MattN+bmo)
Attachment #8865175 - Flags: review?(scwwu)
Attachment #8865175 - Flags: review?(schung)
Attachment #8865175 - Flags: review?(MattN+bmo)
(Assignee)

Comment 15

2 years ago
We're going to address the renaming part in bug 1362911 so clear the review flags.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8865175 - Attachment is obsolete: true
(Assignee)

Comment 18

2 years ago
Patch has been updated base on bug 1362911.
Depends on: 1362911

Comment 19

2 years ago
mozreview-review
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review140504

I only some trivial nits but overall looks fine, thanks.

::: browser/extensions/formautofill/ProfileStorage.jsm:285
(Diff revision 1)
>    _findByGUID(guid) {
> -    return this._store.data.profiles.find(profile => profile.guid == guid);
> -  },
> +    let found = this._findIndexByGUID(guid);
> +    return found < 0 ? undefined : this._store.data[this._dataId][found];
> +  }
> +
> +  _findIndexByGUID(guid) {

Ok, I think having a comment about the main purpose(for sync) might be great.

::: browser/extensions/formautofill/ProfileStorage.jsm:239
(Diff revision 3)
> -   * @returns {Array.<Address>}
> -   *          An array containing clones of all addresses.
> +   * @param   {boolean} config.noComputedFields
> +   *          Returns raw record without those computed fields.
> +   * @returns {Array.<Object>}
> +   *          An array containing clones of all records.
>     */
> -  getAll() {
> +  getAll({noComputedFields} = {}) {

Do we need a test for the case if noComputedFields is provoded?

::: browser/extensions/formautofill/test/unit/test_profileStorage.js:2
(Diff revision 3)
>  /**
> - * Tests ProfileStorage object.
> + * Tests ProfileStorage object with addresses records.

nit: Not sure if you prefer to split the credit card  and addresses records test in different files or into the same file. If you'll create another test file for credit card, maybe you can rename this test to "test_addressesRecords.js". Or rename the task name with more spefic record type like "test_add_address" if you prefer to integrate all the tests in one file.
Attachment #8863062 - Flags: review?(schung) → review+
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review140782

::: browser/extensions/formautofill/ProfileStorage.jsm:51
(Diff revision 5)
> + *     {
> + *       guid,             // 12 characters
> + *
> + *       // credit card fields
> + *       cc-name,
> + *       cc-number,        // encrypted in the disk

Nit: "encrypted on disk"

::: browser/extensions/formautofill/ProfileStorage.jsm:419
(Diff revision 5)
> +  _recordReadProcessor(creditCard, noComputedFields) {
> +    // TODO: Decrypt cc-number here (bug 1337314).

I think it would be better to delay this until it's actually needed so we don't end up prompting for the master password just because we read the file. I think we decided not to filter credit card numbers as you type so ideally we don't actually need to decrypt until the user actually selects a card in autocomplete. The last 4 digits could be stored unencrypted.

To implement this I think we should move to storage returning instances of a CreditCard class rather than vanilla JS objects. Then it can have methods to get the whole number without having to go back to storage but still doing it on-demand.

I think it's a much cleaner architecure that I wanted to move to for addresses too but didn't want to block on switching to ES classes for it.

The "Compute split names" can be done in a lazy getter of this CreditCard class which is cleaner IMO. The constructor could handle raw data from JSONFile and we would still use JSONFile for storage.

What do you think of this approach? I don't think it should be done in this bug but it will affect bug 1337314. This would be similar to nsILoginInfo btw. which has methods like `matches` and `equals` which are also like helpers we're implementing which are better as instance methods.

::: browser/extensions/formautofill/ProfileStorage.jsm:442
(Diff revision 5)
> +      }
> +    }
> +  }
> +
> +  _recordWriteProcessor(creditCard) {
> +    // TODO: Encrypt cc-number here (bug 1337314).

I'm fine with doing this in a follow-up assuming we never save data here before encryption lands.

::: browser/extensions/formautofill/ProfileStorage.jsm:462
(Diff revision 5)
> +    }
> +
> +    // Validate expiry date
> +    if (creditCard["cc-exp-month"]) {
> +      let expMonth = parseInt(creditCard["cc-exp-month"], 10);
> +      if (isNaN(expMonth) || expMonth < 0 || expMonth > 12) {

Don't you want `<= 0`?

::: browser/extensions/formautofill/ProfileStorage.jsm:470
(Diff revision 5)
> +        creditCard["cc-exp-month"] = String(expMonth);
> +      }
> +    }
> +    if (creditCard["cc-exp-year"]) {
> +      let expYear = parseInt(creditCard["cc-exp-year"], 10);
> +      if (isNaN(expYear) || expYear < 0) {

We should probably enforce 4 digits years as a sanity check

::: browser/extensions/formautofill/test/unit/test_creditCardStorage.js:44
(Diff revision 5)
> +let prepareTestCreditCards = Task.async(function* (path) {
> +  let profileStorage = new ProfileStorage(path);

Do we need Task.jsm for this or is there a newer ES way to do this without it? I'm not up-to-date on the async/await stuff.
Attachment #8863083 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review140808
Attachment #8863062 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 24

2 years ago
mozreview-review-reply
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review140782

> I think it would be better to delay this until it's actually needed so we don't end up prompting for the master password just because we read the file. I think we decided not to filter credit card numbers as you type so ideally we don't actually need to decrypt until the user actually selects a card in autocomplete. The last 4 digits could be stored unencrypted.
> 
> To implement this I think we should move to storage returning instances of a CreditCard class rather than vanilla JS objects. Then it can have methods to get the whole number without having to go back to storage but still doing it on-demand.
> 
> I think it's a much cleaner architecure that I wanted to move to for addresses too but didn't want to block on switching to ES classes for it.
> 
> The "Compute split names" can be done in a lazy getter of this CreditCard class which is cleaner IMO. The constructor could handle raw data from JSONFile and we would still use JSONFile for storage.
> 
> What do you think of this approach? I don't think it should be done in this bug but it will affect bug 1337314. This would be similar to nsILoginInfo btw. which has methods like `matches` and `equals` which are also like helpers we're implementing which are better as instance methods.

I've just confirmed with Juwei that we will filter credit card numbers when typing, so we are probably unable to delay the prompt? 

I think there are still benefits of returning instances rather than JS objects, e.g. lazy getter of computed fields. However, I'm not sure if it works as we need to pass the object to the content process via message manager.

> I'm fine with doing this in a follow-up assuming we never save data here before encryption lands.

I'll ensure that encryption will land first.

> Don't you want `<= 0`?

Yup, that's a mistake. Thanks.

> We should probably enforce 4 digits years as a sanity check

Good point!

> Do we need Task.jsm for this or is there a newer ES way to do this without it? I'm not up-to-date on the async/await stuff.

This test file was forked from `test_profileStorage.js` so Task.jsm was kept as-is. I also feel that async/await looks good. Maybe I can try them next time.
(Assignee)

Comment 25

2 years ago
mozreview-review-reply
Comment on attachment 8863062 [details]
Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records.

https://reviewboard.mozilla.org/r/134900/#review140504

> Do we need a test for the case if noComputedFields is provoded?

Tests added.

> nit: Not sure if you prefer to split the credit card  and addresses records test in different files or into the same file. If you'll create another test file for credit card, maybe you can rename this test to "test_addressesRecords.js". Or rename the task name with more spefic record type like "test_add_address" if you prefer to integrate all the tests in one file.

Good Catch. Thanks.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 28

2 years ago
(In reply to Luke Chang [:lchang] from comment #24)
> I've just confirmed with Juwei that we will filter credit card numbers when
> typing, so we are probably unable to delay the prompt? 
> 
> I think there are still benefits of returning instances rather than JS
> objects, e.g. lazy getter of computed fields. However, I'm not sure if it
> works as we need to pass the object to the content process via message
> manager.

Matt, what do you think?
Flags: needinfo?(MattN+bmo)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review140782

> I've just confirmed with Juwei that we will filter credit card numbers when typing, so we are probably unable to delay the prompt? 
> 
> I think there are still benefits of returning instances rather than JS objects, e.g. lazy getter of computed fields. However, I'm not sure if it works as we need to pass the object to the content process via message manager.

We had a meeting and decided that we won't filter credit card numbers and will show unmaksed credit card numbers for users with a master password set (regardless of whether it's logged in or not). Upon selecting a row to fill a credit card, the MP dialog will be forced to appear and upon succesful login the credit card will be filled. Users without a master password will have the whole CC number previewed and will support live-filtering of credit card numbers in autocomplete.

For now we can use a vanilla JS object for now to unblock sync engine progress even though serializing the object won't be a lot of work.
Flags: needinfo?(MattN+bmo)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review141940

::: browser/extensions/formautofill/ProfileStorage.jsm:459
(Diff revision 9)
> +    if (creditCard["cc-number"]) {
> +      let ccNumber = creditCard["cc-number"].replace(/\s/g, "");
> +      delete creditCard["cc-number"];
> +
> +      if (!ccNumber.match(/^\d+$/)) {
> +        throw new Error("Credit card numbers contain invalid characters.");

Nit "Credit cards number contains…" (move the "s")

::: browser/extensions/formautofill/ProfileStorage.jsm:493
(Diff revision 9)
> +    if (creditCard["cc-exp-month"]) {
> +      let expMonth = parseInt(creditCard["cc-exp-month"], 10);
> +      if (isNaN(expMonth) || expMonth < 1 || expMonth > 12) {
> +        delete creditCard["cc-exp-month"];
> +      } else {
> +        creditCard["cc-exp-month"] = String(expMonth);

Why a string here too?

::: browser/extensions/formautofill/ProfileStorage.jsm:500
(Diff revision 9)
> +      } else if (expYear < 100) {
> +        // Enforce 4 digits years.
> +        creditCard["cc-exp-year"] = String(expYear + 2000);

What I meant is that we throw if we are given more or less than 4 digits… I guess this works too. We should  document assumptions in a JSDoc block above "class CreditCards" about what we expect in terms of input to make it clear who is responsible for doing conversions like this.

::: browser/extensions/formautofill/ProfileStorage.jsm:504
(Diff revision 9)
> +        delete creditCard["cc-exp-year"];
> +      } else if (expYear < 100) {
> +        // Enforce 4 digits years.
> +        creditCard["cc-exp-year"] = String(expYear + 2000);
> +      } else {
> +        creditCard["cc-exp-year"] = String(expYear);

Why store a string instead of a number? Are we going to define that we're always going to store strings even for numeric data? I guess it helps with API consistency as long as that's defined and clear.

::: browser/extensions/formautofill/test/unit/test_creditCardRecords.js:79
(Diff revision 9)
> +      do_check_eq(creditCardWithMeta["cc-number-masked"],
> +        "*".repeat(creditCard["cc-number"].length - 4) + creditCard["cc-number"].substr(-4));

Nit: if you're using the same code in the test as the shipping code you're not really testing that this does the right thing…

Comment 35

2 years ago
mozreview-review
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review141952

::: browser/extensions/formautofill/ProfileStorage.jsm:468
(Diff revisions 8 - 9)
> +      // e.g. creditCard["cc-number-encrypted"] = Encrypt(creditCard["cc-number"]);
> +
> +      if (ccNumber.length > 4) {
> +        creditCard["cc-number-masked"] = "*".repeat(ccNumber.length - 4) + ccNumber.substr(-4);
> +      } else {
> +        creditCard["cc-number-masked"] = ccNumber;

Nit: I'm not sure if we need to consider such case since there shouldn't be a valid cc-number that less then 4 digits. Maybe we can file a follow up bug for basic credit card number validation(should be a low priority task)?
Attachment #8863083 - Flags: review?(schung) → review+

Comment 36

2 years ago
mozreview-review-reply
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review141940

> Nit "Credit cards number contains…" (move the "s")

Nit-of-a-nit: I think Matt accidentally introduced another "s" - I think it should read "Credit card number contains…"
(Assignee)

Comment 37

2 years ago
mozreview-review-reply
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review141940

> Nit-of-a-nit: I think Matt accidentally introduced another "s" - I think it should read "Credit card number contains…"

Thanks both of you.

> What I meant is that we throw if we are given more or less than 4 digits… I guess this works too. We should  document assumptions in a JSDoc block above "class CreditCards" about what we expect in terms of input to make it clear who is responsible for doing conversions like this.

I personally would like to make `profileStorage` do the normalization for the most part but I'm not sure if it's the right way. I'll document it first anyway. Since `addresses` should have the same assumption, I'll write it above `class AutofillRecords`.

> Why store a string instead of a number? Are we going to define that we're always going to store strings even for numeric data? I guess it helps with API consistency as long as that's defined and clear.

I intended to covert it to the same type at any time but I forgot why I chose a string rather than a number. After revisiting the `_normalizeRecord` function, I think it was designed to accept both strings and numbers. So I'll change it to store numbers. Thanks for pointint it out.

> Nit: if you're using the same code in the test as the shipping code you're not really testing that this does the right thing…

You're right. Fixed.
(Assignee)

Comment 38

2 years ago
mozreview-review-reply
Comment on attachment 8863083 [details]
Bug 1359978 - (Part 2) Add credit card support to ProfileStorage.jsm.

https://reviewboard.mozilla.org/r/134918/#review141952

> Nit: I'm not sure if we need to consider such case since there shouldn't be a valid cc-number that less then 4 digits. Maybe we can file a follow up bug for basic credit card number validation(should be a low priority task)?

Agree. I'll file a bug to follow up the credit card number validation.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 43

2 years ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 3d9ed00279c9 -d edb50441ee93: rebasing 395763:3d9ed00279c9 "Bug 1359978 - (Part 1) Implement AutofillRecords as a base class for handling address records. r=MattN,steveck"
local [dest] changed browser/extensions/formautofill/test/unit/test_profileStorage.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
merging browser/extensions/formautofill/FormAutofillParent.jsm
merging browser/extensions/formautofill/test/unit/test_enabledStatus.js
merging browser/extensions/formautofill/test/unit/test_savedFieldNames.js
merging browser/extensions/formautofill/test/unit/test_transformFields.js
warning: conflicts while merging browser/extensions/formautofill/test/unit/test_transformFields.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 46

2 years ago
Pushed by lchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/376cb53491aa
(Part 1) Implement AutofillRecords as a base class for handling address records. r=MattN,steveck
https://hg.mozilla.org/integration/autoland/rev/e6f6f7fa0426
(Part 2) Add credit card support to ProfileStorage.jsm. r=MattN,steveck

Comment 47

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/376cb53491aa
https://hg.mozilla.org/mozilla-central/rev/e6f6f7fa0426
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.