Stale shipping options are passed to the front-end after an .updateWith() with no shipping options

RESOLVED FIXED in Firefox 61

Status

()

P1
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: MattN, Assigned: chenyu.chuang)

Tracking

(Blocks: 1 bug)

Trunk
mozilla61
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox61 fixed)

Details

(Whiteboard: [webpayments], URL)

Attachments

(2 attachments, 2 obsolete attachments)

If updateWith is called with no shippingOptions property after there were shipping options, it seems like the shipping options in the parent process (not sure about content process) don't get cleared.

Demo: https://googlechrome.github.io/samples/paymentrequest/dynamic-shipping/ 
* (Remove the legacy `supportedMethods` syntax in the `supportedInstruments` array:
```js
{
    supportedMethods: networks,
},
```
)

There is a TODO in the code that mentions that there is more to do with shippingOptions for updates: https://dxr.mozilla.org/mozilla-central/rev/bccdc684210431c233622650a91454c09f6af9eb/dom/payments/PaymentRequestManager.cpp#546,556
Flags: in-testsuite?
Priority: P1 → P2
Whiteboard: [webpayments]
(Assignee)

Comment 1

9 months ago
The root cause is the spec update, in the old spec, the old values would be kept if they are not in the new details.
I will write a patch according to this spec update.

Could someone assign this bug to me? thanks.
Thank you for taking this.
Assignee: nobody → chenyu.chuang
Status: NEW → ASSIGNED
Priority: P2 → P1
(Assignee)

Comment 3

9 months ago
Created attachment 8963615 [details] [diff] [review]
Bug 1443914 - Force updating PaymentDetails when requestShipping is true. r?baku

Update the implementation according to the spec update.                    
Force updating the saved PaymentDetails when requestShipping is true, even the data(shippingOptions) doesn't present in the DetailsUpdate.
Attachment #8963615 - Flags: review?(amarchesini)
(Assignee)

Comment 4

9 months ago
Created attachment 8963618 [details] [diff] [review]
Bug 1443914 - mochitest for PaymentDetails updating implementation. r?baku
Attachment #8963618 - Flags: review?(amarchesini)
Attachment #8963618 - Flags: review?(amarchesini) → review+
Attachment #8963615 - Flags: review?(amarchesini) → review+
(Assignee)

Comment 6

9 months ago
Hi Baku, thank you for the fast review. :)
(Assignee)

Comment 7

9 months ago
Created attachment 8963707 [details] [diff] [review]
Bug 1443914 - Force updating PaymentDetails when requestShipping is true. r?baku
Attachment #8963615 - Attachment is obsolete: true
Attachment #8963707 - Flags: review+
(Assignee)

Comment 8

9 months ago
Created attachment 8963708 [details] [diff] [review]
Bug 1443914 - mochitest for PaymentDetails updating implementation. r=baku
Attachment #8963618 - Attachment is obsolete: true
Attachment #8963708 - Flags: review+
(Assignee)

Updated

9 months ago
Keywords: checkin-needed

Comment 9

9 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/200ba8d29892
Force updating PaymentDetails when requestShipping is true. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ce3dc676ee9
Mochitest for force updating PaymentDetails. r=baku
Keywords: checkin-needed

Comment 10

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/200ba8d29892
https://hg.mozilla.org/mozilla-central/rev/1ce3dc676ee9
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.