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

Keep WebPush settings #5879

Merged
merged 1 commit into from
Dec 9, 2017
Merged

Conversation

abcang
Copy link
Contributor

@abcang abcang commented Dec 3, 2017

When logging out, the WebPush setting disappears. So, I propose a feature to save the WebPush setting to localStorage and restore it at login.

@ykzts
Copy link
Sponsor Member

ykzts commented Dec 3, 2017

cc @sorin-davidoi

@sorin-davidoi
Copy link
Contributor

Code-wise, I would also remove the setting when logging out. When Safari implements Service Worker and Push Notifications, we might run into this, though we should probably handle it then.

From a technical perspective, I generally don't like this kind of automatic migration of settings between sessions - it is either a global setting or a session setting. There is also a risk of the settings persisting between users (if the session expires in the backend and another users logs in, they will inherit the settings of the previous user). I don't opose it, but I think we should consult more people to see if this kind of behavior is expected.

@abcang
Copy link
Contributor Author

abcang commented Dec 6, 2017

I want to solve this problem because it seems like a bug that WebPush settings disappear when logging out. So, I think that another method that does not use localStorage is acceptable (for example, setting is saved for each account, not for each session). I will close this PR if there are other good solutions.

Also, in this PR, since the setting is saved with the key containing the account id in localStorage, the settings of others are not applied at login.

@Gargron Gargron merged commit 99242b9 into mastodon:master Dec 9, 2017
Gargron pushed a commit that referenced this pull request Dec 9, 2017
* Move push notifications settings

* fix typo `setf` -> `set`
@abcang abcang deleted the keep_webpush_settings branch December 15, 2017 02:11
chriswmartin added a commit to chriswmartin/mastodon that referenced this pull request Dec 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants