Closed
Bug 1318988
Opened 8 years ago
Closed 7 years ago
Implement `allowPaymentRequest` on iframe
Categories
(Core :: DOM: Web Payments, defect, P3)
Core
DOM: Web Payments
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: marcosc, Assigned: alchen)
References
(Blocks 2 open bugs, )
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 3 obsolete files)
12.03 KB,
patch
|
alchen
:
review+
|
Details | Diff | Splinter Review |
Controls if an iframe can access the PaymentRequest API.
Comment 2•7 years ago
|
||
On the spec side: * When a document is initialized (https://html.spec.whatwg.org/#creating-a-new-browsing-context and https://html.spec.whatwg.org/#initialise-the-document-object ), https://html.spec.whatwg.org/#set-the-allow*-flags is run, which can set "allowpaymentrequest flag" and "allowusemedia flag" on Document. * PaymentRequest constructor calls https://html.spec.whatwg.org/#allowed-to-use : https://github.com/w3c/browser-payment-api/pull/383 * getUserMedia also call allowed to use: https://github.com/w3c/mediacapture-main/pull/427 Relevant whatwg/html PRs: https://github.com/whatwg/html/pull/2195 https://github.com/whatwg/html/pull/2217 Relevant w3c/web-platform-tests PRs: https://github.com/w3c/web-platform-tests/pull/4369 https://github.com/w3c/web-platform-tests/pull/4615 I will write more tests for same origin-domain; see https://github.com/w3c/web-platform-tests/issues/4526
Comment 3•7 years ago
|
||
Bug 1318988 has added 'allowPaymentRequest' on iframe, but permission check in PaymentRequest constructor is still required.
Comment 4•7 years ago
|
||
(In reply to Ben Tian [:btian] from comment #3) > Bug 1318988 has added 'allowPaymentRequest' on iframe, but permission check > in PaymentRequest constructor is still required. Should be bug 1331899.
Updated•7 years ago
|
Component: DOM → DOM: Web Payments
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → alchen
Assignee | ||
Comment 5•7 years ago
|
||
We can pass a few web-platform-test by this patch. In this patch, I also add a mochitest.
Assignee | ||
Comment 6•7 years ago
|
||
With this patch, We can pass the following web-platform-tests. https://w3c-test.org/payment-request/allowpaymentrequest/ -> active-document-cross-origin.https.sub.html -> active-document-same-origin.https.html -> allowpaymentrequest-attribute-cross-origin-bc-containers.https.html -> allowpaymentrequest-attribute-same-origin-bc-containers.https.html -> no-attribute-cross-origin-bc-containers.https.html -> no-attribute-same-origin-bc-containers.https.html -> basic.https.html
Attachment #8886064 -
Flags: review?(amarchesini)
Comment 7•7 years ago
|
||
Comment on attachment 8886064 [details] [diff] [review] Implement allowPaymentRequest on iframe Review of attachment 8886064 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/payments/PaymentRequest.cpp @@ +238,5 @@ > + // If the node has the same origin as the parent node, the feature is allowed-to-use. > + // Otherwise, only allow-to-use this feature when the browsing context container is > + // an iframe with "allowpaymentrequest" attribute. > + nsCOMPtr<nsIDocument> doc = window->GetExtantDoc(); > + nsINode* node = static_cast<nsINode*>(doc); node can be null here. In this case return NS_ERROR_UNEXPECTED.
Attachment #8886064 -
Flags: review?(amarchesini) → review+
Updated•7 years ago
|
Blocks: paymentrequest-wpt
Assignee | ||
Comment 8•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b39b0685d800b2a66375d3a3869b6b90057f024
Attachment #8885144 -
Attachment is obsolete: true
Attachment #8886064 -
Attachment is obsolete: true
Attachment #8887302 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 9•7 years ago
|
||
Attachment #8887302 -
Attachment is obsolete: true
Attachment #8887310 -
Flags: review+
Comment 10•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2911ec8bebec Implement allowPaymentRequest on iframe. r=baku
Keywords: checkin-needed
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2911ec8bebec
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 12•7 years ago
|
||
Usually we don't mark features as available in iframes; we are more likely to mark a feature if it is explicitly not available in an iframe. However, I appreciate that this feature's iframe availability is rather more critical than most (3rd party checkout apps, etc.), so I've put a note here about it: https://developer.mozilla.org/en-US/docs/Web/API/Payment_Request_API#Payment_Request_Concepts_and_Usage Do you think anything more is needed?
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #12) > Usually we don't mark features as available in iframes; we are more likely > to mark a feature if it is explicitly not available in an iframe. > > However, I appreciate that this feature's iframe availability is rather more > critical than most (3rd party checkout apps, etc.), so I've put a note here > about it: > > https://developer.mozilla.org/en-US/docs/Web/API/ > Payment_Request_API#Payment_Request_Concepts_and_Usage > > Do you think anything more is needed? I think you should add the note as the link below. It is more accurate. https://www.w3.org/TR/payment-request/#paymentrequest-and-iframe-elements Thanks for your help.
Flags: needinfo?(cmills)
Comment 14•7 years ago
|
||
(In reply to Alphan Chen [:alchen] from comment #13) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #12) > > Usually we don't mark features as available in iframes; we are more likely > > to mark a feature if it is explicitly not available in an iframe. > > > > However, I appreciate that this feature's iframe availability is rather more > > critical than most (3rd party checkout apps, etc.), so I've put a note here > > about it: > > > > https://developer.mozilla.org/en-US/docs/Web/API/ > > Payment_Request_API#Payment_Request_Concepts_and_Usage > > > > Do you think anything more is needed? > > I think you should add the note as the link below. > It is more accurate. > https://www.w3.org/TR/payment-request/#paymentrequest-and-iframe-elements > > Thanks for your help. Ah, cool, thanks for the pointer. I've updated my note so it is accurate, and I've also added an entry to the <iframe> page to explain the allowpaymentrequest attribute. The equivalent DOM property is already documented: https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement
Flags: needinfo?(cmills)
Assignee | ||
Comment 15•7 years ago
|
||
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #14) > > I've updated my note so it is accurate, and I've also added an entry to the > <iframe> page to explain the allowpaymentrequest attribute. > > The equivalent DOM property is already documented: > > https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement There are some updates about this attribute. [Current Definition] To indicate that a "cross-origin" iframe is allowed to invoke the payment request API, the allowpaymentrequest attribute can be specified on the iframe element. The keyword is "cross-origin". If the iframe has the same origin, we don't need 'allowpaymentrequest' attribute to use the payment request API. So we still need some refinement here. Thanks.
Flags: needinfo?(cmills)
Comment 16•7 years ago
|
||
(In reply to Alphan Chen [:alchen] from comment #15) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #14) > > > > I've updated my note so it is accurate, and I've also added an entry to the > > <iframe> page to explain the allowpaymentrequest attribute. > > > > The equivalent DOM property is already documented: > > > > https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement > > There are some updates about this attribute. > > [Current Definition] > To indicate that a "cross-origin" iframe is allowed to invoke the payment > request API, the allowpaymentrequest attribute can be specified on the > iframe element. > > > The keyword is "cross-origin". > If the iframe has the same origin, we don't need 'allowpaymentrequest' > attribute to use the payment request API. > So we still need some refinement here. > Thanks. Ah, I missed that detail - thanks for pointing this out. I've made sure the cross-origin detail is made clear in all the relevant places: * https://developer.mozilla.org/en-US/docs/Web/API/Payment_Request_API#Payment_Request_Concepts_and_Usage * https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement#Properties * https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#Attributes
Flags: needinfo?(cmills)
Updated•7 years ago
|
Whiteboard: [WP-MVP][M4]
Updated•7 years ago
|
Whiteboard: [WP-MVP][M4]
You need to log in
before you can comment on or make changes to this bug.
Description
•