Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reference W3C HTML for allowpaymentrequest definition #359

Merged
merged 3 commits into from Dec 7, 2016

Conversation

sideshowbarker
Copy link
Contributor

@sideshowbarker sideshowbarker commented Dec 5, 2016

Following w3c/html#743, which adds the definition of the <iframe> allowpaymentrequest attribute to W3C HTML, this change updates the W3C Payment Request API to reference W3C HTML for that definition of allowpaymentrequest from W3C HTML.

See also #311

sideshowbarker added a commit to whatwg/html that referenced this pull request Dec 5, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

Closes w3c/payment-request#311
See also w3c/payment-request#359
without this attribute set).
When an <a><code>iframe</code></a> element has an
<a><code>allowpaymentrequest</code></a> attribute, and does not
have any ancestor <a><code>iframe</code></a> element without an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong. This reads like it's talking about ancestor the DOM concept. But you should hook into "allowed to use" instead, which handles nested browsing contexts. See Fullscreen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hook into "allowed to use" instead, which handles nested browsing contexts

Thanks, see 4f59cf9, which attempts that.

requests</a>, then <a>throw</a> a <a>SecurityError</a>.
nested <a>browsing context</a>'s <a>browsing context container</a> is
an <a><code>iframe</code></a> element whose <a>node document<a> is
not allowed to use the feature indicated by attribute name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<a>allowed to use</a>?

nested browsing context is not <a>allowed to make payment
requests</a>, then <a>throw</a> a <a>SecurityError</a>.
nested <a>browsing context</a>'s <a>browsing context container</a> is
an <a><code>iframe</code></a> element whose <a>node document</a> is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if it's not an iframe (e.g. it's frame/object/embed), it wouldn't throw. That's bad. Remove iframe check here I think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we plan to allow nested frame/object/embed to use PaymentRequest? I thought not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. But the current text would allow that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove iframe check here I think.

OK, removed in 681837d

@@ -343,8 +343,11 @@
<li>If the <a>browsing context</a> of the script calling the
constructor is a <a>nested browsing context</a> whose origin is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR should be blocked on fixing these issues:
#324
#332

The whole paragraph doesn't any make sense right now and does not seem actually secure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR should be blocked on fixing these issues:
#324
#332

Agreed. Though I wasn’t sure that this PR itself was the right place to fix those. I guess I imagined this PR branch could be rebased once those fixes got made. But I suppose we could instead make the changes on this branch as part of this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way 😊

sideshowbarker added a commit to whatwg/html that referenced this pull request Dec 5, 2016
w3c/payment-request#359 was updated to hook
into “allowed to use” (like Fullscreen does) in place of the special
“allowed to make payment requests” term the Payment Request had defined
on its own instead.

So this change replaces reference to “allowed to make payment requests” with
“allowed to use the `PaymentRequest` interface to make payment requests”
(just using similar language as for the `allowfullscreen` case).
sideshowbarker added a commit to w3c/html that referenced this pull request Dec 6, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See w3c/payment-request#311
See also w3c/payment-request#359
@sideshowbarker sideshowbarker changed the title Reference HTML for allowpaymentrequest definition Reference W3C HTML for allowpaymentrequest definition Dec 6, 2016
stevefaulkner pushed a commit to w3c/html that referenced this pull request Dec 6, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See w3c/payment-request#311
See also w3c/payment-request#359
Following whatwg/html#2133, which adds the
definition of the <iframe> `allowpaymentrequest` attribute to the HTML
spec, this change updates the Payment Request API to reference HTML for
that definition of `allowpaymentrequest` from HTML.

See also #311
@adrianba adrianba self-assigned this Dec 6, 2016
@adrianba adrianba force-pushed the allowpaymentrequest-move-to-html branch from 681837d to 056e9f4 Compare December 7, 2016 01:25
@adrianba adrianba merged commit bf82c2f into gh-pages Dec 7, 2016
@sideshowbarker sideshowbarker deleted the allowpaymentrequest-move-to-html branch December 7, 2016 03:21
zcorpan pushed a commit to whatwg/html that referenced this pull request Dec 19, 2016
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See
w3c/payment-request#311
w3c/payment-request#359
arronei pushed a commit to arronei/html that referenced this pull request Apr 17, 2017
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See w3c/payment-request#311
See also w3c/payment-request#359
alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
The `allowpaymentrequest` attribute is used by the Payment Request API
to determine if Document objects in an `iframe` element's browsing
context are to be allowed to make payment requests.

See
w3c/payment-request#311
w3c/payment-request#359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants