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

Make it possible to set cookies client-side #25670

Open
morsssss opened this issue Nov 19, 2019 · 24 comments
Open

Make it possible to set cookies client-side #25670

morsssss opened this issue Nov 19, 2019 · 24 comments
Labels

Comments

@morsssss
Copy link
Contributor

We hear this request often from more sophisticated and experienced AMP site creators - to allow reading and setting of cookies from <amp-bind>, <amp-list>, and/or <amp-analytics>.

A common workaround is to set cookies server-side, which sometimes makes it necessary to create an API to set cookies. We want AMP to make things easier for developers, and unfortunately this server-side workaround makes the experience harder for developers and users alike.

Of course, being able to set cookies from <amp-script> would be nice too, and might cover these use cases. But it would be great to add this to older interactive components as, of course, they're far more widely adopted at the moment.

One possible format:

on="tap:AMP.setCookie({currency: 'USD'})"

/cc @mattwomple , @SimonHogstromRonbol, @nainar, @choumx

@alankent
Copy link

Cookie support would be great! Example use cases I have bumped into include:

  • Cookies to remember previous personal info (e.g. the user's name) - on return to the page, need to be able to access the cookie value on page load to inject into page contents (e.g. "Welcome back Alan").
  • Cookies to auto-login the user upon return to the site - logged in users may be shown richer page information, so nice to get that from the first loaded page.
  • Cookies to remember marketing segmentation information - can be useful when sending a URL off to a recommendation engine to return more relevant information based on what is known about the user. (E.g. I will show mountain boots on home page as user is male and likes the outdoors.)

Need to be clear on what restrictions exist for AMP Cache vs origin domain names in URLs and cookies. A cookie set on the origin will be not available if a page is served from the AMP Cache?

Side note: Elsewhere I had wondered if it made sense to associate AMP state with cookies. AMP state is initialized to selected cookies on page load (making it accessible from to render after initial page load). Setting AMP state bound to cookies updated the cookie.

@morsssss
Copy link
Contributor Author

/cc also @mdmower

@nainar , do you think we can prioritize this? Might it even make a Good First Issue™️?

@mdmower
Copy link
Contributor

mdmower commented Feb 21, 2020

Implementing AMP.getCookie() and AMP.setCookie() is easy enough, but a design review may be necessary to determine which directives are configurable. I don't mind starting an I2I if @ampproject/wg-runtime doesn't outright forbid implementation of this feature request. Let me know.

@samouri
Copy link
Member

samouri commented Feb 21, 2020

I don't see a problem with this, but I'm not a good authority on Cookies. @jridgewell, thoughts on allowing AMP.setCookie?

@jridgewell
Copy link
Contributor

Needs a privacy review.

@morsssss
Copy link
Contributor Author

Makes sense. How do we kick that off?

@jridgewell
Copy link
Contributor

Search for SPUR in moma, we have to file ariane tickets, etc.

@alankent
Copy link

I don't know enough about how AMP.getCookie() would be made available, but just a reminder I think a valuable use case is for the cookie to affect rendering of the page on initial page load. E.g. personalization - inject the user's name if returning, or display preferences based on segmentation knowledge from previous sessions etc.

This is why I liked the idea of saying "persist this part of AMP state in a cookie". If the AMP state is updated, update the cookie. On page load, restore that AMP state from the cookie before #26473 kicks in feeding AMP state into amp-list rendering on page load. (I am sure there are other ways of doing it, but getting cookie values on page load into amp-list or similar I think is valuable. I want the original page render affected by the cookie value.)

@morsssss
Copy link
Contributor Author

+1 to that idea too - the ability to serialize a state var into a cookie, then reverse the process on subsequent page load

@mdmower
Copy link
Contributor

mdmower commented Feb 22, 2020

@morsssss @alankent Serialized states are generally too large to store in cookies (rule of thumb is 4K for all cookies on a domain). I think your suggestion would be better accommodated by giving publishers access to localStorage.

@morsssss
Copy link
Contributor Author

I hadn't been thinking about such large state variables! localStorage is an interesting idea...

@mdmower
Copy link
Contributor

mdmower commented Mar 26, 2020

I put together a draft PR for what is probably the simplest approach to adding AMP.setCookie support. I'm finding that cookie support is pretty low on my priority list of interests though, so I'd like to encourage someone else to take on the privacy and design reviews. Feel free to hijack as much or as little of #27397 as you like.

As for getCookie support, my suggestion would be to create a separate I2I. Start simple, like adding cookie('abc') as a supported function in amp-bind (admittedly not trivial... all bind functions evaluate in a worker without access to document). This would allow cookie values to be assigned to states.

@dreamofabear
Copy link

Thanks Matt.

As a meta point, I wonder if promotion of cookies might be a future foot-gun WRT on-cache vs. off-cache, given the direction of 3P cookies e.g. https://webkit.org/blog/10218/full-third-party-cookie-blocking-and-more/. Though there's still value for AMP first/dirty AMP/signed exchanges.

@morsssss
Copy link
Contributor Author

This is a good point. Cookies were tough before in the world of the AMP cache, and I don't think we're going to see signed exchanges everywhere anytime soon.

On the other hand, we are trying hard to promote the idea of AMP-first! And I think the number more of people who will struggle with AMP because they can't use cookies is greater than the number who will have trouble with cookies vanishing on the cache.

So I'm personally in favor of pursuing this. If others aren't, though - if we think cookies are going to become a dinosaur - I think we should simply be public about that in our documentation and outreach.

@morsssss
Copy link
Contributor Author

Noticing this again... is there interest in following this through to completion? It's one of those features that developers frequently request, and it unlocks the door to a number of website features.

Looks like @mdmower gave this a great head start here: #27397

@mdmower
Copy link
Contributor

mdmower commented Aug 24, 2020

@morsssss Out of curiosity, are developers more often requesting "get cookie" or "set cookie" functionality? If "set cookie" is sufficient, it's mostly just a matter of running #27397 through a privacy review (admittedly, not a small task, but requires less design time than a "get cookie" feature).

@morsssss
Copy link
Contributor Author

Sadly my contact with developers isn't what it was before March 🙄 As I think about requests for this that I heard last year and the prior year, I don't feel like I heard people asking for just get or set. I remember requests related to analytics, requests related to persisting state without having to make server calls... and simple client-side personalization. I also remember that @alankent was interested in remembering logged-in state.

If those are still the use cases, "set cookie" will make some of that functionality easier, so it's probably better than nothing :)

Of course it might be easier to do this with amp-script: ampproject/worker-dom#451

@codysaylor
Copy link

You can do a lot with cookies/localstorage, but I'd like to use either to simply remember preferences a user selects such as a preferred dark/light mode css, so I'd love to see this feature implemented.

@morsssss
Copy link
Contributor Author

Makes sense!

@codysaylor , if it helps for the time being, amp-script does support local storage.

@bilalmalkoc
Copy link

Hello. Any news about cookies?

@morsssss
Copy link
Contributor Author

I think we'd need a volunteer to follow up on @mdmower 's exploration.

@mdmower
Copy link
Contributor

mdmower commented Jan 25, 2022

This is a bit speculative, but I imagine the reason cookies-for-AMP hasn't gotten traction is creators would be disappointed by limitations. Reading and writing cookies would likely need to be restricted to source origins (e.g. allowed on example.com, but not on google.com/amp/s/example.com). If all AMP pages served from AMP caches were delivered as signed exchanges (wouldn't that be cool?!), we'd be all set. As it is though, with so many AMP pages viewed from google.com/amp, this cookie feature would lead to differences in capabilities between pages served via an AMP viewer and those served from source origins.

EDIT: I suppose I should have re-read this thread before posting. This concern has been mentioned a few times 😏 . So, nothing new here... but probably valid nonetheless.

@morsssss
Copy link
Contributor Author

it's still an important point @mdmower ! 😎

@stale
Copy link

stale bot commented Jan 21, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Inactive for one year or more label Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants