Add PaymentResponse.prototype.onpayerdetailchange #724

marcoscaceres · 2018-06-12T07:00:22Z

part 4 of #705 - define eventing model for "payerdetailchange".

The following tasks have been completed:

Confirmed there are no ReSpec errors/warnings.
Added Web platform tests
added MDN Docs

Implementation commitment:

Safari
Chrome (link to issue)
Firefox
Edge (public signal)

Impact on Payment Handler spec?

Preview | Diff

ianbjacobs · 2018-06-14T21:13:48Z

@marcoscaceres, it has been pointed out that the proposal as written (if I read the code correctly) requires the developer to query all the fields to find out which thing changed. What would be your suggested good practice for figuring out which thing changed?

marcoscaceres · 2018-06-14T21:27:03Z

What would be your suggested good practice for figuring out which thing changed?

There is not way to detect which changed, just revalidate. So the question is mostly around validation of these values...

response.onpayerdetailchange = ev => {
    const errors = {};
    const { payerName, payerEmail, payerPhone } = response; 
    object.assign(
       errors, validateEmail(payerEmail), validateName(payerName), validatePhone(payerPhone) 
    );
   if(Object.getOwnPropertyNames(x).length){
      response.retry(errors);
   }
}

marcoscaceres · 2018-06-14T21:30:44Z

actually, to be safe, maybe the should be separate events 🧐... if each of those requires a network lookup for validation, then this could get a little bit sad. Question is, is anyone doing this validation over the network?

stpeter · 2018-06-14T21:35:35Z

@marcoscaceres What would the separate events be?

marcoscaceres · 2018-06-14T21:49:44Z

onpayeremailchange, onpayerphonechange, and onpayernamechange.

stpeter · 2018-06-14T21:51:19Z

That's what I was afraid of, but I see why we might need it...

marcoscaceres · 2018-06-14T22:01:31Z

The problem is that if UI autofills the three fields at the same time, we get into a race condition.

ianbjacobs · 2018-06-14T22:03:25Z

Ah, I had not thought about the autofill scenario; I was only imagining independent manual interaction.

marcoscaceres · 2018-06-14T22:05:25Z

Discussed with our UI folks too. The single event is better for us, because it gives us more flexibility as to when we fire the event (e.g., clicking "next").

marcoscaceres · 2018-06-14T22:27:26Z

so, this is how one would handle having the single event:

let {
  payerName: oldPayerName,
  payerEmail: oldPayerEmail,
  payerPhone: oldPayerPhone,
} = response;
response.onpayerdetailchange = async ev => {
  const promisesToValidate = [];
  const { payerName, payerEmail, payerPhone } = response;
  if (oldPayerName !== payerName) {
    promisesToValidate.push(validateName(payerName));
    oldPayerName = payerName;
  }
  if (oldPayerEmail !== payerEmail) {
    promisesToValidate.push(validateEmail(payerEmail));
    oldPayerEmail = payerEmail;
  }
  if (oldPayerPhone !== payerPhone) {
    promisesToValidate.push(validatePhone(payerPhone));
    oldPayerPhone = payerPhone;
  }
  const errors = await Promise.all(promisesToValidate).then(results =>
    results.reduce((errors, result), Object.assign(errors, result))
  );
  if (Object.getOwnPropertyNames(errors).length) {
    await response.retry(errors);
  }
};

DannyRussellWP · 2018-06-18T09:16:40Z

Based on my experience of implementing payments request on multiple demo's for worldpay now. I would echo the recommendation of marco's ui guys and leave the single event. While a complicated handler is not perfect I admit it does keep the code a lot clearer than having a lot of separate event handlers

ianbjacobs · 2018-06-25T15:15:05Z

Hi @marcoscaceres,

I have been asking around for views on the question of "one event" v. "multiple events". You can see that @DannyRussellWP supported a single event. I've heard back similarly from two other payments companies as well.

Ian

marcoscaceres · 2018-06-25T15:29:27Z

Excellent, thanks for the update @ianbjacobs.

wanli · 2018-06-25T19:55:04Z

@marcoscaceres @ianbjacobs - The preference from Airbnb is that one event handler would suffice.

I asked internally and have updated this action regarding this:
https://www.w3.org/Payments/WG/track/actions/97

marcoscaceres · 2018-06-25T22:29:25Z

Appreciate the feedback @wanli!

domenic

A little hard to review due to the switched headings... let's get that fixed.

domenic · 2018-06-26T21:38:53Z

index.html

+            "DOM#event-target">target</a> is an instance of
+            <a>PaymentResponse</a>, let <var>request</var> be
+            <var>event</var>'s <a data-cite=
+            "DOM#event-target">target</a><a>[[\request]]</a>.


Missing dot

domenic · 2018-06-26T21:39:15Z

index.html

          <li>Let <var>request</var> be the <a>PaymentRequest</a> object that
          the user is interacting with.
          </li>
+          <li>If <var>request</var><a>[[\response]]</a> is null, terminate this


Missing dot

Use "return" instead of "terminate this algorithm"

domenic · 2018-06-26T21:39:22Z

index.html

+          algorithm.
+          </li>
+          <li>Let <var>response</var> be
+          <var>request</var><a>[[\response]]</a>.


Missing dot

Move this step up so that you can check if response is null.

domenic · 2018-06-26T21:40:50Z

index.html

+              <li>If <var>payer name</var> changed:
+                <ol>
+                  <li data-link-for="PaymentOptions">Assert:
+                  <var>request</var><a>[[\options]]</a>.<a>requestPayerName</a>


Search for </var><a>[[, so many missing dots...

domenic · 2018-06-26T21:44:12Z

index.html

          with:
        </p>
        <ol class="algorithm">
          <li>Let <var>request</var> be the <a>PaymentRequest</a> object that


The algorithms seem to be switched; this is the payment details changed algorithm.

Argh, oops... screwed up the merge/

marcoscaceres · 2018-06-28T18:58:05Z

@domenic, hopefully less bad now 🤞

domenic

LGTM!

domenic · 2018-06-28T19:30:59Z

index.html

          with:
        </p>
-        <ol class="algorithm">
+                <ol class="algorithm">


Whitspace change seems bad

domenic · 2018-06-28T19:32:44Z

index.html

+          the user is interacting with.
+          </li>
+          <li>If <var>request</var>.<a>[[\response]]</a> is null, terminate
+          this algorithm.


"terminate this algorithm" -> "return"

domenic · 2018-06-28T19:33:55Z

index.html

+              <li data-link-for="PaymentRequestUpdateEvent">If
+              <var>event</var>.<a>[[\waitForUpdate]]</a> is true, disable any
+              part of the user interface that could cause another update event
+              to be fired.


Is "another update event" appropriate here? Maybe "another change to the payer details"?

marcoscaceres · 2018-06-28T22:14:11Z

Added Gecko tracking bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1472026

marcoscaceres · 2018-06-28T22:44:13Z

Dev docs added https://developer.mozilla.org/en-US/docs/Web/API/PaymentResponse/onpayerdetailchange

marcoscaceres · 2018-07-03T17:02:26Z

Added tests web-platform-tests/wpt#11772

marcoscaceres · 2018-07-03T17:03:47Z

Firefox implementation underway.

The event handler is triggered when the user changes payer information such as payer name, payer email, or payer phone in the user interface. Related spec change: w3c/payment-request#724 Test: payment-request/payment-response/onpayerdetailchange-attribute.https.html payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html Bug: 861704 Change-Id: Ia5d63f53874abd7c76014bf35379a71a0eead622

…2846) The event handler is triggered when the user changes payer information such as payer name, payer email, or payer phone in the user interface. Related spec change: w3c/payment-request#724 Test: payment-request/payment-response/onpayerdetailchange-attribute.https.html payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html Bug: 861704 Change-Id: Ia5d63f53874abd7c76014bf35379a71a0eead622

…e.payerdetailchange event, a=testonly Automatic update from web-platform-testsPaymentRequest: Implement PaymentResponse.payerdetailchange event (#12846) The event handler is triggered when the user changes payer information such as payer name, payer email, or payer phone in the user interface. Related spec change: w3c/payment-request#724 Test: payment-request/payment-response/onpayerdetailchange-attribute.https.html payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html Bug: 861704 Change-Id: Ia5d63f53874abd7c76014bf35379a71a0eead622 -- wpt-commits: 3c6c9350988f98c8fac94fab8174bc4729cd6755 wpt-pr: 12846

The event handler is triggered when the user changes payer information such as payer name, payer email, or payer phone in the user interface. Unlike other PaymentRequestUpdateEvent(e.g. shippingaddresschange), this event can fire only after the website calls retry() with validation errors in the payer contact information. This feature is still behind runtime flag(PaymentRetry). Intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/wayZGnuBkrI Related spec change: w3c/payment-request#724 Test: payment-request/payment-response/onpayerdetailchange-attribute.https.html payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html Bug: 861704 Change-Id: Ia5d63f53874abd7c76014bf35379a71a0eead622 Reviewed-on: https://chromium-review.googlesource.com/1206750 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org> Commit-Queue: Jinho Bang <jinho.bang@samsung.com> Cr-Commit-Position: refs/heads/master@{#590961}

aestes · 2018-11-02T21:56:25Z

WebKit tracking bug: https://bugs.webkit.org/show_bug.cgi?id=189249

marcoscaceres · 2018-11-05T02:46:33Z

@romandev, any news on this one (PaymentResponse.prototype.onpayerdetailchange) from the Chrome side?

romandev · 2018-11-05T02:50:52Z

Ah, I didn't create a new separate bug but it's already finished here:
https://chromium-review.googlesource.com/c/chromium/src/+/1206750

romandev · 2018-11-05T02:58:12Z

Chrome tracking bug: https://bugs.chromium.org/p/chromium/issues/detail?id=901710

marcoscaceres · 2018-11-05T03:19:25Z

Awesome. Looks like I need to do another run of WTP to update the implementation report.

The event handler is triggered when the user changes payer information such as payer name, payer email, or payer phone in the user interface. Unlike other PaymentRequestUpdateEvent(e.g. shippingaddresschange), this event can fire only after the website calls retry() with validation errors in the payer contact information. This feature is still behind runtime flag(PaymentRetry). FYI, we've already had a desktop implementation in crrev.com/c/1206750. Intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/wayZGnuBkrI Related spec change: w3c/payment-request#724 Test: chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestRetryTest.java payment-request/payment-response/onpayerdetailchange-attribute.https.html payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html Bug: 861704 Change-Id: If9d01e4696b0ed8e415a76313c70da4d6ec230f6 Reviewed-on: https://chromium-review.googlesource.com/c/1477610 Commit-Queue: Jinho Bang <jinho.bang@samsung.com> Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org> Cr-Commit-Position: refs/heads/master@{#636489}

…e.payerdetailchange event, a=testonly Automatic update from web-platform-testsPaymentRequest: Implement PaymentResponse.payerdetailchange event (#12846) The event handler is triggered when the user changes payer information such as payer name, payer email, or payer phone in the user interface. Related spec change: w3c/payment-request#724 Test: payment-request/payment-response/onpayerdetailchange-attribute.https.html payment-request/payment-response/onpayerdetailchange-attribute.manual.https.html Bug: 861704 Change-Id: Ia5d63f53874abd7c76014bf35379a71a0eead622 -- wpt-commits: 3c6c9350988f98c8fac94fab8174bc4729cd6755 wpt-pr: 12846 UltraBlame original commit: 9d6bcd31919749a8a6cb9fe5d2ca3fa37a6a28ba

marcoscaceres added the Blocked label Jun 12, 2018

marcoscaceres force-pushed the retry_request_events branch from e6f78f9 to f448712 Compare June 25, 2018 23:42

Define onpayerdetailchange event

1dfd596

marcoscaceres force-pushed the retry_request_events branch from f448712 to 1dfd596 Compare June 26, 2018 20:58

marcoscaceres requested a review from domenic June 26, 2018 20:59

marcoscaceres changed the title ~~Add PaymentResponse.onpayerdetailchange~~ Add paymentResponse.prototype.onpayerdetailchange Jun 26, 2018

marcoscaceres changed the title ~~Add paymentResponse.prototype.onpayerdetailchange~~ Add PaymentResponse.prototype.onpayerdetailchange Jun 26, 2018

domenic requested changes Jun 26, 2018

View reviewed changes

marcoscaceres added 2 commits June 28, 2018 13:34

dots

5ee6c00

fix bad merge

185721f

domenic approved these changes Jun 28, 2018

View reviewed changes

Review feedback

397c3b6

marcoscaceres mentioned this pull request Jul 3, 2018

tests for onpayerdetailchange web-platform-tests/wpt#11772

Merged

marcoscaceres merged commit d27cc16 into gh-pages Jul 3, 2018

marcoscaceres deleted the retry_request_events branch July 3, 2018 17:02

chromium-wpt-export-bot mentioned this pull request Sep 5, 2018

PaymentRequest: Implement PaymentResponse.payerdetailchange event web-platform-tests/wpt#12846

Merged

Add PaymentResponse.prototype.onpayerdetailchange #724

Add PaymentResponse.prototype.onpayerdetailchange #724

Uh oh!

Conversation

marcoscaceres commented Jun 12, 2018 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianbjacobs commented Jun 14, 2018

Uh oh!

marcoscaceres commented Jun 14, 2018

Uh oh!

marcoscaceres commented Jun 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stpeter commented Jun 14, 2018

Uh oh!

marcoscaceres commented Jun 14, 2018

Uh oh!

stpeter commented Jun 14, 2018

Uh oh!

marcoscaceres commented Jun 14, 2018

Uh oh!

ianbjacobs commented Jun 14, 2018

Uh oh!

marcoscaceres commented Jun 14, 2018

Uh oh!

marcoscaceres commented Jun 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DannyRussellWP commented Jun 18, 2018

Uh oh!

ianbjacobs commented Jun 25, 2018

Uh oh!

marcoscaceres commented Jun 25, 2018

Uh oh!

wanli commented Jun 25, 2018

Uh oh!

marcoscaceres commented Jun 25, 2018

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoscaceres commented Jun 28, 2018

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoscaceres commented Jun 28, 2018

Uh oh!

marcoscaceres commented Jun 28, 2018

Uh oh!

marcoscaceres commented Jul 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcoscaceres commented Jul 3, 2018

Uh oh!

aestes commented Nov 2, 2018

Uh oh!

marcoscaceres commented Nov 5, 2018

Uh oh!

romandev commented Nov 5, 2018

Uh oh!

marcoscaceres commented Jun 12, 2018 •

edited by pr-preview bot

Loading

marcoscaceres commented Jun 14, 2018 •

edited

Loading

marcoscaceres commented Jun 14, 2018 •

edited

Loading

marcoscaceres commented Jul 3, 2018 •

edited

Loading

romandev commented Nov 5, 2018 •

edited

Loading