With multiple tabs open at the same time, when the cookie compliance is accepted on one tab, the other isn't aware of the cookie state change and keeps wiping all cookies until the page is reloaded or the specific tab closed.
This is particularly annoying when you have multiple tabs already open, and need to start a new xdebug session on one, you end up having to hunt down the tab which is clearing the cookies.
clearInterval(euCookieComplianceBlockCookies); is only called when the accept action is triggered.
The var euCookieComplianceBlockCookies = setInterval(function () { call should check if the cookie has been accepted before blocking the cookies, if it's been accepted, then the interval check should be cancelled from there.
N/A
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3143538-interdiff-3-11.txt | 2.74 KB | codebymikey |
| #11 | 3143538-11.eu_cookie_compliance.Cookies-are-always-wiped-when-multiple-tabs-are-open.patch | 2.21 KB | codebymikey |
Comments
Comment #2
svenryen commentedNice catch. Are you interested in making a patch?
Comment #3
codebymikey commentedAttached was my initial patch which addressed my specific use case with regards to canceling the interval.
It's not perfect since it doesn't do the rest of the logic which
Drupal.eu_cookie_compliance.acceptActiontypically does such as callingeuCookieComplianceLoadScriptsorDrupal.eu_cookie_compliance.loadCategoryScripts.But I didn't want to duplicate or refactor the code since I wasn't currently using those options and wasn't sure about the nuance of potentially loading scripts across multiple background tabs - which would be a change in behaviour over what it's been doing for years now. Open to feedback if that's an acceptable change in behaviour, I personally think it's probably fine to load them in the background.
I removed the var in the
var euCookieComplianceBlockCookiesdeclaration because it was already declared at the top of the script.Comment #5
svenryen commentedThanks for the patch. If somebody still needs the acceptAction and loadCategoryScripts, hopefully they will open a new issue.
Comment #6
codebymikey commentedThanks for merging it, one thing I noticed after reviewing my submitted patch logic was the lack of a
return;statement. Meaning that the code clears the interval callback, but might still end up wiping the current cookies one final time.The bug is not apparent unless a third party cookie is quickly set in between intervals - which is pretty rare, but should still be addressed nonetheless.
I meant to raise the issue/resubmit the patch after I heard your opinion on the
euCookieComplianceLoadScriptsissue, but given the time that's gone between then and now, it fell through the cracks.Comment #7
svenryen commentedI see. Should I revert your patch, or will you make a new one based on the current -dev? You can open a follow-up issue if you want.
Comment #8
codebymikey commentedI think you can keep it in, the patch is still functional, and definitely better than the old behaviour. Just thought I'd make you aware of the race condition issue that can occur.
I'm not currently working today, so will try to reopen a issue for it or just update the patch here when I next work with
eu_cookie_compliance.Comment #10
svenryen commented@codebymikey - The issue was automatically closed. Do we want to reopen it, or how do you propose we proceed?
Comment #11
codebymikey commented#3130015: Write & document public JS API for actions & events introduced a change or
regression in behaviour of the
Drupal.eu_cookie_compliance.hasAgreedfunction.I'm not sure if this was intentional, but this means that the
hasAgreedcall no longer reads off the cookies, butinstead on a local variable state (which can only be updated when the user clicks the relevant button).
I've rerolled the patch introducing a new
Drupal.eu_cookie_compliance.getCookieStatusfunction which aims to read the cookie value withoutnecessarily updating the
_euccCurrentStatusvariable to work around this change in behaviour.But it'll need further thought from you and other maintainers with regards to the best way to handle this going forward.
Comment #12
svenryen commentedComment #14
svenryen commentedComment #16
svenryen commented