[Affected versions]: Nightly 64.0a1 [Affected platforms]: Platforms: Windows 10 x64, Ubuntu 18.04, Mac OS 10.13 [Preconditions]: 1. Set the pref dom.payments.request.enabled to "true"; 2. Make sure you are using a clean profile. [Steps to reproduce]: 1. Go to "https://rsolomakhin.github.io/pr/single/" page and click on "Buy". 2. Fill "Add shipping address" from correctly. 3. Fill "Add Credit Card" form correctly. 4. Close the payment dialog. 5. Go to "about:preferences" -> Privacy & Security -> Forms & Autofill. 6. Click on "Saved Credit Cards.." and delete the CC introduced before. 7. Go back to "https://rsolomakhin.github.io/pr/single/" page and click on "Buy". 8. Fill "Add Credit Card" form correctly. [Expected result]: CC should be saved without any errors. [Actual result]: CC is saved even when "There was an error saving the payment card." message is displayed.

I found new steps to reproduce this issue: 1. Make sure you are on a FTU flow. 2. Go to "https://rsolomakhin.github.io/pr/single/" page and click on "Buy". 3. Fill "Add shipping address" from correctly. 4. From "Add Credit Card" form click on "Cancel" or on the "X" button. 5. From "https://rsolomakhin.github.io/pr/single/" page and click on "Buy" again. 6. Fill "Add Credit Card" form correctly. 7. Observe the error message displayed.

So if I turned on logging I can see the error is caused by this error https://searchfox.org/mozilla-central/rev/65f9687eb192f8317b4e02b0b791932eff6237cc/browser/components/payments/res/containers/basic-card-form.js#425,435 It is unclear to me where selectedStateKey is set, and what is supposed to be. Matt, is that something obvious to you? If not I will keep debugging.

Wait, I've just realized the comment 1 linked to the other thing I am working on, which is totally unrelated to payment. Timea, would you be able to check the regression range again? I backed out the bugs (and their dependencies) locally and I can still reproduce this bug. (Leaving unassigned for now) @ 489509:31af1457befc timdream tip Backed out changeset 50d1b7c5b1a1 o 489508:ba8caabbafce timdream Backed out changeset d8dca67e2440 o 489507:578b0fd3e5d0 timdream Backed out changeset 4392b5198fb7 o 489506:a979b65806ad timdream Backed out changeset a632efc6cac4 o 489505:96549b50926d timdream Backed out changeset 039c4b2029a4 o 489504:20773596530b apavel central Backed out changeset 39f0d4e66898 (bug 675428) for causing Bug 1496380 a=backout

Hi Tim, Thanks for checking it on your side! I checked it again with mozregression using the steps from Comment 2 (makes it easier to repro) and you were right. I found the issue that is actually Payments related, makes sense now. Sorry for the inconvenience created before :( Last good revision: 39762ef5d56e07f8046bf9dfb1b32f194c6ea1bd First bad revision: 4ff0fcd61c7a1f791414516a6e22efca94530f19 https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=39762ef5d56e07f8046bf9dfb1b32f194c6ea1bd&tochange=4ff0fcd61c7a1f791414516a6e22efca94530f19 Bug 1490805 - Add the CVV security code field to the add card form and make it required in all places Matt, can you please take a look at this?

I took a look at this bug. As pointed out in the bug description, the error happens at: https://hg.mozilla.org/integration/autoland/rev/4ff0fcd61c7a#l3.71 The first time the basic credit card form is shown, selectedStateKey is initialized to 'selectedPaymentCard'. The initialization happens at: https://hg.mozilla.org/integration/autoland/rev/4ff0fcd61c7a#l2.12 The second time, since an address already exists, selectedStateKey is not initialized. In fact, its value is initialized to null at: https://hg.mozilla.org/integration/autoland/rev/4ff0fcd61c7a#l5.12 And since its value is null an Error is thrown (https://hg.mozilla.org/integration/autoland/rev/4ff0fcd61c7a#l3.71). If basicCardPage.selectedStateKey is initialized to 'selectedPaymentCard' in PaymentStateSubscriberMixin.js the error doesn't happen. That's what the attached patch does to avoid the bug. But I'm not sure that's the right fix.

Comment on attachment 9017523 [details] [diff] [review] Bug-1497514-Initialize-BasicCardPage-selectedStateKe.patch Review of attachment 9017523 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/payments/res/mixins/PaymentStateSubscriberMixin.js @@ +16,5 @@ > orderDetailsShowing: false, > "basic-card-page": { > guid: null, > // preserveFieldValues: true, > + selectedStateKey: "selectPaymentCard", Can you find the place where the page.id is set to "basic-card-page" and set this there instead?

Updated patch.

Comment on attachment 9017822 [details] [diff] [review] Bug-1497514-Initialize-BasicCardPage-selectedStateKe.patch Review of attachment 9017822 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! That's what I was looking for. Now can you add the following after the page.id check[1] in basic-card-form to prevent this from happening again by causing test failures: ```js if (!basicCardPage.selectedStateKey) { throw new Error("A `selectedStateKey` is required"); } ``` Then run all tests and ensure they pass or modify then to add a selectedStateKey: > ./mach test browser/components/payments/ [1] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/browser/components/payments/res/containers/basic-card-form.js#138-142

I've added a new patch adding the check suggested to `basic-card-form.js`. After that, I run the tests and there was one set of tests that reported several errors. All the fails where in one single test: `test/mochitest/test_basic_card_form.html`. I tried to fix the tests by adding a `selectedStateKey: "selectedPaymentCard"` to all `basic-card-page` state objects defined (or defining it if necessary). Now the tests don't fail due to the extra check added in `basic-card-form.js`, but report an error in function `checkCCForm` (https://searchfox.org/mozilla-central/source/browser/components/payments/test/mochitest/test_basic_card_form.html#47). It seems that when the function tries to fetch a property from the document, that property doesn't exist. I tried to fix the issue, but no success yet.

Comment on attachment 9018197 [details] [diff] [review] Bug-1497514-Check-basicCardPage-has-a-selectedStateK.patch Review of attachment 9018197 [details] [diff] [review]: ----------------------------------------------------------------- I changed my mind that it's fine to change the default value as long as we have the new `throw`.

Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/mozilla-inbound/rev/8b3b53341da2 Initialize BasicCardPage selectedStateKey to 'selectPaymentCard'. r=MattN

Backed out changeset 8b3b53341da2 (Bug 1497514) for failures in browser/components/payments/test/mochitest/test_basic_card_form.html Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=206496270&revision=9c7087c217509cc8ad7186295e12410264f3b40a Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=206496270&repo=mozilla-inbound&lineNumber=2172 Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/5f7e9756a2efa098d8e0d0e3a4d4447da0d0f1fd

It looks like the failure is only with QuantumRender… Diego, can you import https://hg.mozilla.org/integration/mozilla-inbound/rev/8b3b53341da2 and debug the failure using Try (or locally if you can run Quantum Render)? There is probably an existing race in the test.

I imported the patch and run the test. It seems some tests that were using "basic-card-page" needed to be initialized with "selectedStateKey: selectedPaymentCard". After that change, the test passed.

Comment on attachment 9018523 [details] [diff] [review] Bug-1497514-Initialize-BasicCardPage-selectedStateKe.patch Review of attachment 9018523 [details] [diff] [review]: ----------------------------------------------------------------- Ah, yeah, of course. Thanks!

Are you able to push to try server?

Yes, I can (it slipped my mind). Treeherder: https://treeherder.mozilla.org/#/jobs?repo=try&revision=abda8b03a1114b35db11ec4036ac9106d049c040

Pushed by ebalazs@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/25594c3013c4 Initialize BasicCardPage selectedStateKey to 'selectPaymentCard'. r=MattN

Verified as fixed on Firefox Nightly 64.0a1 (2018-10-21) on Windows 10 x 64, Windows 7 x32, Mac OS X 10.13 and on Ubuntu 16.04 x64.