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

✨ Add "crossorigin" attribute to remote xhr elements to attach id token. #21107

Merged
merged 10 commits into from Mar 14, 2019

Conversation

hellokoji
Copy link
Contributor

Implement Issue (#18787).

Adds the cross-origin attribute to amp-state, amp-list, and amp-form. Given the presence of this attribute, the remote XHR requests will be transformed into POST requests and attach an identity token provided by the amp-viewer-assistance extension.

@choumx

Change-Id: If2c56a3cc32ccfe97efb64572dbaf86fdb1d54d0
Change-Id: Ia74748cea663883663f0121e4fda9bed0f7b9d8d
@dreamofabear dreamofabear self-requested a review February 26, 2019 21:17
Change-Id: If19b962e606828b6efd4308b5870480be1c973e9
extensions/amp-bind/0.1/amp-state.js Outdated Show resolved Hide resolved
extensions/amp-bind/0.1/amp-state.js Outdated Show resolved Hide resolved
extensions/amp-bind/0.1/amp-state.js Outdated Show resolved Hide resolved
src/batched-json.js Outdated Show resolved Hide resolved
extensions/amp-list/0.1/amp-list.js Outdated Show resolved Hide resolved
@hellokoji hellokoji changed the title ✨ Add "cross-origin" attribute to remote xhr elements to attach id token. ✨ Add "crossorigin" attribute to remote xhr elements to attach id token. Mar 7, 2019
Change-Id: I8e9410492f018bdbf9702e8cf37e8c1b1a2a9a51
Change-Id: I2f7dea01e553c5a38f6ff3d6c5116d50ea62f438
Change-Id: Iea2f29fc25ba9de982eb1cc4bc3c3d9459c754ee
Change-Id: I6859a879e1bee170627f1c06fd473da0bc3d2fd5
@dreamofabear
Copy link

/cc @honeybadgerdontcare for validator changes.

@honeybadgerdontcare
Copy link
Contributor

Validation looks good. Only consideration would be to put in a deprecation_url to a GitHub issue explaining why it's being deprecated.

Change-Id: I510c0778a60db259486d55b116961d1173198402
@hellokoji
Copy link
Contributor Author

Created Issue (#21399) and added it as the deprecation_url in the most recent commit.

Copy link

@dreamofabear dreamofabear left a comment

Choose a reason for hiding this comment

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

Just one comment and a couple nits, otherwise LGTM.

extensions/amp-bind/0.1/amp-state.js Outdated Show resolved Hide resolved
extensions/amp-form/0.1/amp-form.js Outdated Show resolved Hide resolved
src/services.js Outdated Show resolved Hide resolved
Change-Id: Id301b039be182e4c3f92de3b790357edea21a340
Change-Id: I3b04db719fd41f72475e0686aa71e6dd5a0b43fe
Copy link

@dreamofabear dreamofabear left a comment

Choose a reason for hiding this comment

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

Looks nice and clean, good work!

src/utils/xhr-utils.js Show resolved Hide resolved
@dreamofabear dreamofabear merged commit 76a0c2c into ampproject:master Mar 14, 2019
Gregable pushed a commit that referenced this pull request Mar 20, 2019
 - cl/239414441 Revision bump for #21463
 - cl/239234304 Add deprecation warning for manufactured body caused by non-ASCII whitespace.
 - cl/239228383 Explicitly disallow `<template>` outside the document `<body>`.
 - cl/239220081 Add a feature test case for a non-amp document.
 - cl/239050986 Revision bump for #21459
 - cl/239048160 Fix corner case of ancestor marker tracking.
 - cl/238487968 Revision bump for #21107
 - cl/238483735 Revision bump for #21264
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
…en. (ampproject#21107)

* Add "cross-origin" attribute to remote xhr elements to attach id token.

Change-Id: If2c56a3cc32ccfe97efb64572dbaf86fdb1d54d0

* Lint.

Change-Id: Ia74748cea663883663f0121e4fda9bed0f7b9d8d

* Address check-types, and presubmits.

Change-Id: If19b962e606828b6efd4308b5870480be1c973e9

* Address PR Comments 1.

Change-Id: I8e9410492f018bdbf9702e8cf37e8c1b1a2a9a51

* Fix tests.

Change-Id: Iea2f29fc25ba9de982eb1cc4bc3c3d9459c754ee

* Update validator rules to support crossorigin.

Change-Id: I6859a879e1bee170627f1c06fd473da0bc3d2fd5

* Add deprecation URL and update validator tests.

Change-Id: I510c0778a60db259486d55b116961d1173198402

* Address PR Comments 2.

Change-Id: Id301b039be182e4c3f92de3b790357edea21a340

* Change serviceForDoc impl to getElementServiceIfAvailableForDoc.

Change-Id: I3b04db719fd41f72475e0686aa71e6dd5a0b43fe
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
 - cl/239414441 Revision bump for ampproject#21463
 - cl/239234304 Add deprecation warning for manufactured body caused by non-ASCII whitespace.
 - cl/239228383 Explicitly disallow `<template>` outside the document `<body>`.
 - cl/239220081 Add a feature test case for a non-amp document.
 - cl/239050986 Revision bump for ampproject#21459
 - cl/239048160 Fix corner case of ancestor marker tracking.
 - cl/238487968 Revision bump for ampproject#21107
 - cl/238483735 Revision bump for ampproject#21264
bramanudom pushed a commit to bramanudom/amphtml that referenced this pull request Mar 22, 2019
…en. (ampproject#21107)

* Add "cross-origin" attribute to remote xhr elements to attach id token.

Change-Id: If2c56a3cc32ccfe97efb64572dbaf86fdb1d54d0

* Lint.

Change-Id: Ia74748cea663883663f0121e4fda9bed0f7b9d8d

* Address check-types, and presubmits.

Change-Id: If19b962e606828b6efd4308b5870480be1c973e9

* Address PR Comments 1.

Change-Id: I8e9410492f018bdbf9702e8cf37e8c1b1a2a9a51

* Fix tests.

Change-Id: Iea2f29fc25ba9de982eb1cc4bc3c3d9459c754ee

* Update validator rules to support crossorigin.

Change-Id: I6859a879e1bee170627f1c06fd473da0bc3d2fd5

* Add deprecation URL and update validator tests.

Change-Id: I510c0778a60db259486d55b116961d1173198402

* Address PR Comments 2.

Change-Id: Id301b039be182e4c3f92de3b790357edea21a340

* Change serviceForDoc impl to getElementServiceIfAvailableForDoc.

Change-Id: I3b04db719fd41f72475e0686aa71e6dd5a0b43fe
bramanudom pushed a commit to bramanudom/amphtml that referenced this pull request Mar 22, 2019
 - cl/239414441 Revision bump for ampproject#21463
 - cl/239234304 Add deprecation warning for manufactured body caused by non-ASCII whitespace.
 - cl/239228383 Explicitly disallow `<template>` outside the document `<body>`.
 - cl/239220081 Add a feature test case for a non-amp document.
 - cl/239050986 Revision bump for ampproject#21459
 - cl/239048160 Fix corner case of ancestor marker tracking.
 - cl/238487968 Revision bump for ampproject#21107
 - cl/238483735 Revision bump for ampproject#21264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants