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

📖 Amp-analytics: Document feature to write cookie to origin domain #19590

Merged
merged 2 commits into from Dec 4, 2018

Conversation

zhouyx
Copy link
Contributor

@zhouyx zhouyx commented Dec 3, 2018

Closes #19589

@zhouyx
Copy link
Contributor Author

zhouyx commented Dec 3, 2018

cc @lannka

Copy link
Member

@calebcordry calebcordry left a comment

Choose a reason for hiding this comment

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

Nice work! a few nits.

@@ -695,6 +695,12 @@ Detials on setting up your linker configuration are outlined in [Linker ID Forwa

If you need to ingest this paramter, information on how this parameter is created is illistrated in [Linker ID Receiving](./linker-id-receiving.md).

#### Cookies

The `cookies` feature supports writing cookies to the origin domain by extracting [`QUERY_PARAM`](https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md#query-parameter) and [`LINKER_PARAM`](./linker-id-receiving.md#linker-param) information from the document url. It can be used along with `linkers` features to perform ID syncing from AMP proxy domain to AMP pages on the publisher domain.
Copy link
Member

Choose a reason for hiding this comment

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

...syncing from the AMP proxied domain to AMP pages on a publisher's domain.


#### Configuration

The `cookies` example configuration looks like
Copy link
Member

Choose a reason for hiding this comment

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

s/The/A

The `enabled` value can be used to override default vendor settings.

##### Cookie Names
Each key value within the `cookies` config object defines the cookie key, where its value needs to be an object.
Copy link
Member

Choose a reason for hiding this comment

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

Each key within the cookies config object defines the cookie name. It's value needs to be an object containing a single key value pair. That key should be 'value', and its value should be the macro that determines the information stored in the cookie.

This is a really hard sentence because the keys name is value. Hopefully someone else will help improve :)

##### Cookie Values
Each cookie to write is defined by an object, where the value is defined by the required `value` field.

Two Macros are supported for the `value` field. [`QUERY_PARAM`](https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md#query-parameter) and [`LINKER_PARAM`](#linker-param).
Copy link
Member

Choose a reason for hiding this comment

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

macros


Two Macros are supported for the `value` field. [`QUERY_PARAM`](https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md#query-parameter) and [`LINKER_PARAM`](#linker-param).

When there's error resoving the value, or the value is resolved to empty string. Nothing will be written to the cookie.
Copy link
Member

Choose a reason for hiding this comment

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

s/when/if

When there's error resoving the value, or the value is resolved to empty string. Nothing will be written to the cookie.

##### LINKER PARAM
`LINKER_PARAM` takes two params, the `name` and the `value`. It will verify the checksum and read the `idName1*idValue1` key value pair from linker param. The `value` ('idValue1' here) will be returned.
Copy link
Member

Choose a reason for hiding this comment

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

s/params/arguments
s/from linker param/ from the linker param

@calebcordry
Copy link
Member

also cc/ @CrystalFaith for docs magic

@zhouyx
Copy link
Contributor Author

zhouyx commented Dec 4, 2018

@calebcordry Thank you for reviewing! Addressed comments.

@zhouyx zhouyx merged commit ebf4e77 into ampproject:master Dec 4, 2018
@zhouyx zhouyx deleted the cookie-writer-doc branch December 4, 2018 21:29
dreamofabear pushed a commit that referenced this pull request Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants