1428472 - Default to the selected shipping address (if applicable) on the "add basic card" screen

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

When a user chooses to use a new payment card (not from existing storage), provide a checkbox to use the shipping address previously selected from summary page for the new card's billing address.

The selected shipping address should already be in the dialog state object and can be referenced when the "add" view is opened.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1428472 - Add a 'Use same as shipping' checkbox to the summary screen.

Review commit: https://reviewboard.mozilla.org/r/233476/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233476/

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8964749 [details]
Bug 1428472 - Add a 'Use same as shipping' checkbox to the summary screen.

https://reviewboard.mozilla.org/r/233476/#review239398

After looking at the direction for bug 1432952 more, it seems like most of this patch would need changes so I'm not sure it makes sense to land/review as-is. Building on top of bug 1432952 (at least part 1 there) would make sense to me or at least try to head in that direction more e.g. we wouldn't need two more state properties. Sorry to make you re-do work but I don't think it makes sense to land as-is.

::: toolkit/components/payments/res/components/address-option.css:49
(Diff revision 1)
> -address-picker.shipping-related address-option > .email,
> -address-picker.shipping-related address-option.rich-select-selected-clone > .tel {
> +address-picker:-moz-any(.billing-related, .shipping-related) address-option > .email,
> +address-picker:-moz-any(.billing-related, .shipping-related) address-option.rich-select-selected-clone > .tel {

We should probably make sure that we only give the merchant what the user expects so e.g. we shouldn't give the telephone number to the merchant if it's hidden.

Comment 3

[Matthew N. [:MattN]]

After thinking about this checkbox more and discussing with Jacqueline, we don't think it's necessary (see IRC discussion for details). Instead we will simply default to the shipping address (if applicable) when showing the form to add a new basic card.

https://mozilla.logbot.info/payments/20180413#c14606956-c14607042

Comment 4

[Matthew N. [:MattN]]

Attachment: Bug 1428472 - Default to the selected shipping address on the "add basic card" screen.

Review commit: https://reviewboard.mozilla.org/r/236608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/236608/

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8967913 [details]
Bug 1428472 - Default to the selected shipping address on the "add basic card" screen.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/236608/diff/1-2/

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8967913 [details]
Bug 1428472 - Default to the selected shipping address on the "add basic card" screen.

https://reviewboard.mozilla.org/r/236608/#review242806

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a72193c7fb1
Default to the selected shipping address on the "add basic card" screen. r=jaws

Comment 8

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 4a72193c7fb1 (bug 1428472) for bc7 and TV failures on a CLOSED TREE

Problematic push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4a72193c7fb1e0c7baaa9676e2f0728fcd1093d2&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified
Failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=173969864
Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d541b42ce4d949434020d4fcc13a9f3807c5e76e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified
Log: https://treeherder.mozilla.org/logviewer.html#?job_id=173969864&repo=autoland&lineNumber=6743

[task 2018-04-16T21:26:40.610Z] 21:26:40     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/payments/test/browser/browser_change_shipping.js | Test timed out - 
[task 2018-04-16T21:26:40.612Z] 21:26:40     INFO - GECKO(3299) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2018-04-16T21:26:40.614Z] 21:26:40     INFO - GECKO(3299) | MEMORY STAT | vsize 2505MB | residentFast 519MB | heapAllocated 294MB
[task 2018-04-16T21:26:40.615Z] 21:26:40     INFO - TEST-OK | toolkit/components/payments/test/browser/browser_change_shipping.js | took 90194ms
[task 2018-04-16T21:26:40.617Z] 21:26:40     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-04-16T21:26:40.618Z] 21:26:40     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/payments/test/browser/browser_change_shipping.js | Found a tab after previous test timed out: https://example.com/browser/toolkit/components/payments/test/browser/blank_page.html - 
[task 2018-04-16T21:26:40.619Z] 21:26:40     INFO - GECKO(3299) | ++DOCSHELL 0x7f73a1686000 == 1 [pid = 3458] [id = {670824ec-6a1e-477f-81a5-74ffb2d72758}]

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69383ad18409
Default to the selected shipping address on the "add basic card" screen. r=jaws

Comment 10

[Matthew N. [:MattN]]

This TV failure is an existing QuantumRender TV issue affecting mochitest-browser-chrome tests using BrowserTestUtils.withNewTab/openNewForegroundTab (such as bug 1454333). I'm going to investigate more and file that as a separate bug since it's not related to web payments.

Comment 11

[Matthew N. [:MattN]]

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #10)
> This TV failure is an existing QuantumRender TV issue affecting
> mochitest-browser-chrome tests using
> BrowserTestUtils.withNewTab/openNewForegroundTab (such as bug 1454333). I'm
> going to investigate more and file that as a separate bug since it's not
> related to web payments.

I filed bug 1454531.

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/69383ad18409