Bug 217365 – [Payment Request] Calling PaymentRequest's show() should consume user activation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Marcos Caceres]

See: 
https://github.com/w3c/payment-request/pull/916

Tests incoming via WPT.

[Radar WebKit Bug Importer]

<rdar://problem/70238596>

[Marcos Caceres]

Created attachment 436153 [details]
Patch

[Marcos Caceres]

Created attachment 436374 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Marcos Caceres]

Created attachment 436375 [details]
Patch

[Darin Adler]

Comment on attachment 436375 [details]
Patch

Patch needs to update or delete this file:

LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/payment-request/show-method-optional-promise-rejects.https-expected.txt

[Marcos Caceres]

Created attachment 436443 [details]
Patch

[youenn fablet]

Comment on attachment 436443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436443&action=review

> Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp:375
> +    if (window && !window->consumeTransientActivation()) {

Should it be:
if (!window || !window->consumeTransientActivation()) {
}

> LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-is-showing.https.html:7
> +<meta name="timeout" content="long" />

I am guessing this is resyncing our copy of WPT tests with upstream.
If so, can you update the change log?

[Marcos Caceres]

Comment on attachment 436443 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436443&action=review

>> Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp:375
>> +    if (window && !window->consumeTransientActivation()) {
> 
> Should it be:
> if (!window || !window->consumeTransientActivation()) {
> }

Yes. You are right. I was unsure if I should treat a nullprt window as a different kind of error. Will update.

>> LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-is-showing.https.html:7
>> +<meta name="timeout" content="long" />
> 
> I am guessing this is resyncing our copy of WPT tests with upstream.
> If so, can you update the change log?

Will do!

[Marcos Caceres]

Created attachment 436477 [details]
Patch

[Devin Rousso]

Comment on attachment 436477 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436477&action=review

r=me as well :)

> LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https-expected.txt:4
> +FAIL Throws if the promise [[state]] is not 'created'. promise_rejects_dom: function "function () { throw e }" threw object "SecurityError: show() must be triggered by user activation." that is not a DOMException InvalidStateError: property "code" is equal to 18, expected 11

Is there a reason that we're still failing?

[Marcos Caceres]

(In reply to Devin Rousso from comment #11)
> > LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https-expected.txt:4
> > +FAIL Throws if the promise [[state]] is not 'created'. promise_rejects_dom: function "function () { throw e }" threw object "SecurityError: show() must be triggered by user activation." that is not a DOMException InvalidStateError: property "code" is equal to 18, expected 11
> 
> Is there a reason that we're still failing?

Just returning to this now... investigating the failures now.

[Marcos Caceres]

Created attachment 436859 [details]
Patch

[Marcos Caceres]

(In reply to Marcos Caceres from comment #12)
> (In reply to Devin Rousso from comment #11)
> > Is there a reason that we're still failing?

Bad test. I sent a PR to fix it on the WPT side:
https://github.com/web-platform-tests/wpt/pull/30255
 
And all PASS now! ^_^

> Just returning to this now... investigating the failures now.

Hopefully all good now. 🤞

[EWS]

Committed r281830 (241163@main): <https://commits.webkit.org/241163@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 436859 [details].

[Brent Fulgham]

This change should be present in iOS 15.1, and macOS 12.1 or newer.