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
a year ago
a year 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

a year 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

a year ago
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)
Attachment #8963618 - Flags: review?(amarchesini) → review+
Attachment #8963615 - Flags: review?(amarchesini) → review+
(Assignee)

Comment 6

a year ago
Hi Baku, thank you for the fast review. :)
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 9

a year 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
https://hg.mozilla.org/mozilla-central/rev/200ba8d29892
https://hg.mozilla.org/mozilla-central/rev/1ce3dc676ee9
Status: ASSIGNED → RESOLVED
Last Resolved: a year 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.