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.

Sign up for GitHub

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

Jump to bottom

Change client id substitution to set a fallback cookie if not present. #1627

Merged
merged 1 commit into from
Jan 28, 2016
Merged

Change client id substitution to set a fallback cookie if not present. #1627

merged 1 commit into from
Jan 28, 2016

Conversation

cramforce
Copy link
Member

No description provided.

@cramforce cramforce force-pushed the analytics-set-cookies branch 3 times, most recently from 9154547 to 3b0c750 Compare January 27, 2016 23:11
@cramforce
Copy link
Member Author

CC @btownsend @avimehta @rudygalfi

@avimehta
Copy link
Contributor

@cramforce How does this work in relation to #1628? If a person specifies a data-consent-notification-id but no user-consent-id in client id, will the client id be written? If a person does not specify data-consent-notification-id but adds a user-consent-id, will the cookie be written? Will analytics tag fire in either of the cases?

Finally, How are vendors supposed to specify user-consent-id? They probably don't have access to the publisher page.

@cramforce cramforce force-pushed the analytics-set-cookies branch 5 times, most recently from 46335bf to 1ec3586 Compare January 28, 2016 19:09
@cramforce
Copy link
Member Author

@avimehta I addressed your question in the other thread. CLIENT_ID supports passing in a consent id, but for amp-analytics and amp-ad (where the users don't touch the CLIENT_ID themselves) there is a special attribute on the tag to guard it based on consent.

@cramforce
Copy link
Member Author

@dvoytenko PTAL

@dvoytenko
Copy link
Contributor

LGTM

@cramforce cramforce force-pushed the analytics-set-cookies branch from 1ec3586 to 56a4876 Compare January 28, 2016 22:38
@cramforce cramforce force-pushed the analytics-set-cookies branch from 56a4876 to 7fa6534 Compare January 28, 2016 22:42
@cramforce cramforce changed the title Provide additional client id substitution that opts into setting the fallback cookie if not present. Change client id substitution to set a fallback cookie if not present. Jan 28, 2016
cramforce added a commit that referenced this pull request Jan 28, 2016
@cramforce
Merge pull request #1627 from cramforce/analytics-set-cookies
30261d4 
Change client id substitution to set a fallback cookie if not present.
@cramforce cramforce merged commit 30261d4 into ampproject:master Jan 28, 2016
@cramforce cramforce deleted the analytics-set-cookies branch January 28, 2016 22:53
@avimehta avimehta mentioned this pull request Feb 2, 2016
Closed
@rudygalfi rudygalfi mentioned this pull request Feb 2, 2016
Closed
@elijahsoria elijahsoria mentioned this pull request Apr 28, 2021
Merged
This was referenced Aug 27, 2022
Open
Open
@snyk-bot snyk-bot mentioned this pull request Aug 27, 2022
Open
This was referenced Oct 4, 2022
Open
Open
Open
This was referenced Mar 17, 2023
Open
Open
Open
This was referenced Sep 5, 2023
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dvoytenko dvoytenko

Labels
WG: analytics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cramforce @avimehta @dvoytenko