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

don't refresh on overwrite #25

Merged
merged 1 commit into from Nov 9, 2020
Merged

don't refresh on overwrite #25

merged 1 commit into from Nov 9, 2020

Conversation

@fcjr
Copy link
Member

@fcjr fcjr commented Nov 6, 2020

Refreshing on overwrite could cause the extension to keep trying to refresh the token when it does not need to, @sammacbeth not 100% sure but we might want to also not destroy it if the remove cause is overwrite as well?

@fcjr fcjr requested review from chrmod and sammacbeth and removed request for chrmod Nov 6, 2020
@sammacbeth
Copy link
Contributor

@sammacbeth sammacbeth commented Nov 9, 2020

Refreshing on overwrite could cause the extension to keep trying to refresh the token when it does not need to, @sammacbeth not 100% sure but we might want to also not destroy it if the remove cause is overwrite as well?

From reading the onChanged docs this looks to be the correct approx. A cookie removal with the cause as overwrite will be immediately followed by an updated cookie value.

@sammacbeth sammacbeth merged commit 7836057 into master Nov 9, 2020
@chrmod chrmod deleted the refresh branch Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants