search

widgetti / solara

A Pure Python, React-style Framework for Scaling Your Jupyter and Web Apps
https://solara.dev
MIT License
1.62k stars 105 forks source link

fix dev cookie on Safari #234

Open qingant opened 9 months ago

qingant commented 9 months ago

The behavior to set Secure=True when serving from localhost would cause failure to set cookie on Safari (tested on 17.0) and thus cause it hang on loading animation.

This could be an issue of Safari but pretty amount of developer who work on Mac would be prevented when they first try Solara on Mac if Safari was set as default browser.

maartenbreddels commented 9 months ago

If this is really happening on Safarai 17 I think this is a browser bug (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value says localhost is ok) , I do not have this issue on Safari 16.5.2.

But, as the comment explains, this is needed for iframe support, so we cannot merge this as is. We'll need to think how we can work around this issue, maybe some opt out?

qingant commented 9 months ago

If this is really happening on Safarai 17 I think this is a browser bug (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value says localhost is ok) , I do not have this issue on Safari 16.5.2.

But, as the comment explains, this is needed for iframe support, so we cannot merge this as is. We'll need to think how we can work around this issue, maybe some opt out?

thanks. could you please point me to the direction where I could add the command options ? or , change to localStorage when Safari detected ? recent days, there is a couple of bugs around cookie in Safari (cookie manager for streamlit is also broken on Safari.)

qingant commented 9 months ago

@maartenbreddels

seems there is already an open issue: https://github.com/widgetti/solara/issues/133

It actually didn't work on my Safari 16.x version. I am guessing the reason that your browser works is because you already have cookie populated from previous run.

maartenbreddels commented 9 months ago

You are correct, if i remove my cookie I get the same issue. This is pretty bad!

It seems we are doing a smart default thing (samesite=none and secure=true) to make more things work out of the box, but this default smart behaviour does not work on safari indeed. I think we need to have something like

  1. A way to detect Safari (seems sth like if 'Safari' in user_agent and 'Chrome' not in user_agent and 'Chromium' not in user_agent:). If safari, do not do the smart default.

  2. manually turn on or off the samesite=none and secure=true settings. Turning it on can be useful if we are behind a proxy and fail to detect we are using https.

    I usually start adding these kinds of options in the solara.server.settings module. If they are common I also expose it via the command line (I don't think we need it this time).

What do you think of this?

maartenbreddels commented 6 months ago

This is 'kinda of' of replaced by #379 @iisakkirotko I think this means that developing with safari on localhost still would not give a stable session id, so .. we should still keep this PR open as a reminder we can still improve...