1478029 - The standard shipping option (default) is “null” for the worldwide multi-option shipping without pre-selection

Status: VERIFIED FIXED

(Firefox :: WebPayments UI, defect, P1)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Gif. of the issue

[Affected versions]:
Nightly 63.0a1

[Affected platforms]:
Windows 10 x64, Mac OS X 10.13, Ubuntu 18.04 x64

[Prerequisites]:
- set the pref dom.payments.request.enabled to "true"
- use a profile with saved cards

[Steps to reproduce]:
1. Go to “https://rsolomakhin.github.io/pr/expl/” and click on “Buy” button
2. Select a saved shipping address and payment method
3. Check if the shipping option is pre-selected with “$0.00 Standrd shipping”
4. Fill in the CVV code and click on “Pay”

[Expected Result]:
The response for the test should correctly reflect the shipping option as "shippingOption": standard.


[Actual Result]:
The "shippingOption": null response is displayed.

Comment 1

[Matthew N. [:MattN]]

Hmm… either bug 1440041 regressed or the front-end is resetting the shipping option. Could you please set the pref dom.payments.loglevel to "Debug" and attach the Browser Console logs? Please clear the logs before you click Buy. You should disable CSS/XHR/Requests logging from the filter toggle but enable all the others: https://cl.ly/401Q0o1j0O28

Please then attach the logs as an attachment. Then we can look if the front-end sent a "changeShippingOption" message.

Thanks!

Comment 2

[Timea Cernea [:tbabos][inactive]]

Hey Matthew,

I attached the browser console log as you requested, hope it helps.
Please ask for any further information that is needed.

Comment 3

[Timea Cernea [:tbabos][inactive]]

Attachment: Browser Console Log for issue

Comment 4

[Matthew N. [:MattN]]

I don't see a "changeShippingOption" in the log so I think this is a follow-up to bug 1440041.

Comment 5

[Eden Chuang[:edenchuang]]

The merchant does set any shippingOption as a pre-select/default shippingOption when constructing the PaymentRequest. Therefore nsIPaymentRequest is created with empty shippingOption. According to the spec

https://www.w3.org/TR/payment-request/#dom-paymentdetailsbase-shippingoptions

there should be no pre-selected/deafult shipping option under the case.

When nsIPaymentRequest is passed to UI, I guess UI shows the first element of shippingOptions as the default selection while nsIPaymentRequest.shippingOption is empty. That's the reason why you see shippingOption "$0.00 Standrd shipping" in the dialog.

So I would say showing the first shippingOpton might not be the behavior fitting the spec. If we want to do it in UI, UI must fire the changeShippingOption event first.

Comment 6

[Matthew N. [:MattN]]

See bug 1440041 comment 16 for some other context, specially option (A).

(In reply to Eden Chuang[:edenchuang] from comment #5)
> The merchant does set any shippingOption as a pre-select/default
> shippingOption when constructing the PaymentRequest. Therefore
> nsIPaymentRequest is created with empty shippingOption. According to the spec
> 
> https://www.w3.org/TR/payment-request/#dom-paymentdetailsbase-shippingoptions
> 
> there should be no pre-selected/deafult shipping option under the case.
> 
> When nsIPaymentRequest is passed to UI, I guess UI shows the first element
> of shippingOptions as the default selection while
> nsIPaymentRequest.shippingOption is empty. That's the reason why you see
> shippingOption "$0.00 Standrd shipping" in the dialog.

Right, but we want this logic moved to the DOM so that the DOM and UI behaviour matches without having to wait for a shippingoptionchange event dispatch.

> So I would say showing the first shippingOpton might not be the behavior
> fitting the spec. If we want to do it in UI, UI must fire the
> changeShippingOption event first.

Or the DOM could before the UI is shown so that the UI doesn't need to wait for the even to be dispatched immediately upon showing. I think that would be better IMO but I'm open to discussing this more.

Comment 7

[Eden Chuang[:edenchuang]]

I am OK with moving the logic to the DOM, but it might make WPT-test fail. 

Marcos, How do you think?

Comment 8

[Marcos Caceres [:marcosc]]

(In reply to Eden Chuang[:edenchuang] from comment #7)
> I am OK with moving the logic to the DOM, but it might make WPT-test fail. 
> 
> Marcos, How do you think?

My reading of the spec is, if there is no merchant pre-"selected" shipping option, the user must choose in the UI: even if there is only 1 option to select from. That's a merchant controlled thing (or error). 

Thus, I'm not sure the DOM code should correct for that (so I don't think DOM code should fire the event). I understand that that means a not super great user experience, but that's on the merchant.

Hopefully I've understood the discussion correctly.

Comment 9

[Eden Chuang[:edenchuang]]

(In reply to Marcos Caceres [:marcosc] from comment #8)
> (In reply to Eden Chuang[:edenchuang] from comment #7)
> > I am OK with moving the logic to the DOM, but it might make WPT-test fail. 
> > 
> > Marcos, How do you think?
> 
> My reading of the spec is, if there is no merchant pre-"selected" shipping
> option, the user must choose in the UI: even if there is only 1 option to
> select from. That's a merchant controlled thing (or error). 
> 
> Thus, I'm not sure the DOM code should correct for that (so I don't think
> DOM code should fire the event). I understand that that means a not super
> great user experience, but that's on the merchant.
> 
> Hopefully I've understood the discussion correctly.

Hmm, the logic seems that we make a decision for users, no matter it is done in DOM or in UI.
I am not sure this is good for users, because they might not know we do it for them.

From the implementation aspect, even the logic is implemented in DOM, DOM still fires the shippingOptionChange event to the merchant, then UI updates the new value from the merchant.

Comment 10

[Matthew N. [:MattN]]

I think WPT tests would only fail if they assume that a user takes a certain amount of time to change the shipping option because we could/should implement the shippingOptionChange event dispatching in .show(), not at construction time.

I talked through this with Jacqueline and these are her preferences in order from best to worst:
A) Select the first shipping option and dispatch the shippingOptionChange event before any UI is shown.
B) Don't pre-select a shipping option when none are selected.
C) Not acceptable: Select the first shipping option and dispatch the shippingOptionChange event after UI is shown. The user may temporarily see a stale total.

I guess a similar problem may occur when the shipping options change (e.g. one gets added) from updateWith and we were planning to remember the user's previous selection. We would need to do a shipping option change in that case to get the right total.

Since the current state is broken and doing (A) is tricky, I think we should implement (B) for now so we can enable on Nightly with expected behaviour from the merchant perspective then we can improve that to get (A) later (it will be easier to do from the front-end when we have our final dialog widget so we can delay showing the UI until the event round-trips).

I guess that means that this is a front-end bug for now and I'll file follow-ups to potentially improve this UX post-M3.

Jacqueline also had the idea of communicating with developers when they're causing a less than ideal UX. I suggested we can display console warnings in some cases e.g. only one (or >0) shipping option but none selected by default.

Comment 11

[Marcos Caceres [:marcosc]]

Im strongly in favor of, “B) Don't pre-select a shipping option...”. The rationale being that if the merchant wants something selected, they have the means to set the right flag. I also agree with Matt’s rationale. 

We can maybe add a console warning, reminding developers that not setting a default option degrades the user experience.

Comment 12

[Matthew N. [:MattN]]

(In reply to Marcos Caceres [:marcosc] from comment #11)
> We can maybe add a console warning, reminding developers that not setting a
> default option degrades the user experience.

I filed bug 1484068 for that but forgot to link it here.

Comment 13

[Marcos Caceres [:marcosc]]

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #12)
> I filed bug 1484068 for that but forgot to link it here.

Ok, but 1484068 reads like you'd like us to do "A)"? You are ok with us only doing "B)" + console.warn for now, right?

Comment 14

[Matthew N. [:MattN]]

(In reply to Marcos Caceres [:marcosc] from comment #13)
> (In reply to Matthew N. [:MattN] (PM if requests are blocking you) from
> comment #12)
> > I filed bug 1484068 for that but forgot to link it here.
> 
> Ok, but 1484068 reads like you'd like us to do "A)"? You are ok with us only
> doing "B)" + console.warn for now, right?

Yes, UX still prefers (A) but I will do (B) for now as it will fix our current issues and (A) is complicated to do now.

Comment 15

[Matthew N. [:MattN]]

Attachment: Bug 1478029 - Don't select a shipping option by default for the user. r=sfoster

Honour the .selected value provided by the merchant.

Comment 16

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 9002129 [details]
Bug 1478029 - Don't select a shipping option by default for the user. r=sfoster

Sam Foster [:sfoster] has approved the revision.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd0770bb6b0d
Don't select a shipping option by default for the user. r=sfoster

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b8443626934
Follow-up to fix test_address_form.html default country. r=me

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/cd0770bb6b0d
https://hg.mozilla.org/mozilla-central/rev/3b8443626934

Comment 20

[Timea Cernea [:tbabos][inactive]]

Verified - Fixed on all OS as:  B) Don't pre-select a shipping option when none are selected.