🇳🇿New Zealand @atowl

atowl → created an issue.

Hi @eueco

Thanks for the very detailed report! I like how much information you supplied!!

I feel however, that this is probably more to do with the google_analytics module? The EUCC module isn't responsible for cleaning up the analytics cookies left behind, and if we do, that's just for the analytics cookies, what about other sites cookies?

We do provide notifiers inside the JS for other modules to hook into to see the state of categories accepted? maybe thats what you're after instead?

Very happy to discuss this further if you think we need to look into it

thanks!

Hi @guillaumeg,

I've attached a patch of the MR, if you could just confirm that works i'll look to merge it for the next release.

Thanks!

Many thanks @irafah for getting back to me on that, will closed this as fixed.

Hi @irafah

Is this still an issue for you on the later versions?

Hi @wotts,

If you modify the libraries file to look like

eu_cookie_compliance:
  js:
    js/eu_cookie_compliance.js: { minified: false }

you can debug/change your tab index, then execute `npm run uglify` to get the minified output.
Or just add your change to the issue fork, and i can take care of the minification

Hi @malirezaei

Are you still seeing this error at all with the later version of the EUCC?

If you need to debug it a bit better, you could change the eu_cookie_compliance.libraries.yml to look like:

eu_cookie_compliance:
  js:
    js/eu_cookie_compliance.js: { minified: false }

then perform a Cache clear, and it should pinpoint a better line for your error.

Closing this as a duplicate, looks like we fixed it in your other issue 🐛 Javascript cache bust variable remained constant Active

Closing this as fixed, looks like it's been merged already.

Hi All,
Merged and closed as fixed.

Thanks for help on this issue.

For a reproduction so far i have tried
- install drupal/google_tag^1.8
- set up a new container (my_first_container), all fake-ish data and defaults.
- changed to opt-in with categories
- created 2 categories, one for my testing, and one for analytics (both unchecked by default).
- added my disabled javascripts

test_module:modules/custom/test_module/js/console_log.js|test_module
analytics:https://www.googletagmanager.com/gtag.js
analytics:sites/default/files/google_tag/my_first_container/google_tag.script.js

Now accepting the analytics category i don't see either of the two scripts pop up, however ever, applying @guillaumeg's patch, i see the both appear.

I think this is the reproduction that i'm looking for.

Hi @pianomansam,

All merged, thanks for your time on this issue!

Hi @guillaumeg, and @elaman,

I've started a new issue about this 🐛 Javascript libraries attached in HEAD not loaded Active , and also attached @guillaumeg's patch to it.
We can discuss it further over there.

Uploading the patch that was supplied in the previous issue 🐛 disabled scripts not being included after consent Active from @guillaumeg. It is the same file, just renamed to suit this issue so it can be discussed here.

atowl → created an issue.

Hi @joachim,

Think we need more information, and possibly a good reproduction to see whats going on here.

Hi @pianomansam

So i've spent some time going over this, and i think i've tested enough to make sure this works, just making sure since this will affect performance if we do get it wrong :)

In my ddev env turned on enough caching to get the X-Drupal-Cache to have a 'HIT' on a 2nd page refresh.
(For those playing along at home i had to use

$settings['cache']['bins']['render'] = 'cache.backend.database';
$settings['cache']['bins']['page'] = 'cache.backend.database';
$settings['cache']['bins']['dynamic_page_cache'] = 'cache.backend.database';

)

I then took your patch, and placed a log message after it, cleared the caches (drush cr), and refreshed the page.
First page load after cache clearing, i saw X-Drupal-Cache: MISS, and the log message.
Every then every subsequent page load was a X-Drupal-Cache: HIT, and no log message, as it's obviously coming from the cache.

Is that what you were expecting/tested?

Hi @pianomansam,

Great idea. Thanks.

atowl → created an issue.

So just to continue on the conversation that @berdir has kicked off, if headers:DNT is now deprecated, what do we put there instead?

While I don't see an issue in continuing with this patch at present, but just wondered if there was something else to put in? at the same time, or maybe we create a new issue and deal with it there.

OK thanks for the information @pianomansam,

I'll (re) close this as fixed, and i'll look at merging #3480626, and any further disucussion in that thread.

Hi @jino matthew,

From my brief search, to meet CCPA requirements, you may need to:

-Include a “Do Not Sell My Personal Information” link.
-Provide clear disclosures about data collection and usage.
-Allow users to opt out of data sharing/selling.
-Ensure data access and deletion mechanisms are in place.

The EUCC by default goes for the opt-in strategy, where as CCPA is more opt-out from what i read.

Hope this helps.

Hi @brtamas,

With the recent releases, could i get you to check this again to see if it's still the case?

Thanks!

Hi @saarlandtoday,

Would like to help you out on this one, can you give me more information about how you're doing this? I'd like to setup a reproduction if possible.

thanks!

Hey Dan,

I'm probably not going to merge #3408084 until the whole thing is ready, as it will turn all the pipelines red. Would rather have it all working and merge it across.

I'll merge the parent into this one.

Thanks everyone for your help on this issue,

I've merged this in, will be available in the next release shortly

Hi @adriancid,
Have merged this in, thanks for your work on the issue.

Hi @guillaumeg,

Yes, all the testing i did for this issue i used drupal/google_tag^2.0.8 (to be specific).

So you are using the 1.8 version? I can try again with that.

Hi @lmoeni

Is MR141 good to go here?

As there's no real information here, i'll close this, please feel free to reopen the issue with more information, or join us in #eu_cookie_compliance on slack for more help.

Hi @adriancid,

I've switched this across to 8.x-1.x.

atowl → made their first commit to this issue’s fork.

Hi All,

Is the MR138 good to go? or are we still missing something?
It looks fine to me.

atowl → made their first commit to this issue’s fork.

Hi,

Is this still an issue? i tried unsuccessfully to reproduce this and it didn't work.
Are you still able to reproduce it?

Hi All,

As a new maintainer of this module, i've copied svenryen's spreadsheet, and struck out anything that has been fixed so far.
https://docs.google.com/spreadsheets/d/e/2PACX-1vRHj1rtlZ3Jxp6v4S0wz__eC...

There's a channel i also have created on slack #eu_cookie_compliance, where we can chat about things.
@rajeshreeputra I have been attempting to contact you, but seems you're not available on slack or email?

If others are interested in joining in as maintainers, i'd appreciate some action in some issues first. I am slowly wading through these. but would appreciate the help.

That being said, i'll leave this issue along for a little while longer before closing it.

Hi @joe_carvajal,

That defer is for the disabled scripts loader, so only gets enabled if there's something in the disabled scripts.

After doing some reading, i'm not totally convinced yet that it would make much of a difference? Are you seeing any significant improvements by doing this?

It could be an issue with drupal.behaviors, as the async wouldn't wait (fired straight after first file download), so its possible that if a disabled script is trying to enable a behavior then it might not work.

Would be keen to hear more if anyone else has opinions before making the change, OR we could make it a configuration to be set?

Closing this issue as i feel we have enough advertisement around the possible migration to klaro now.

Closing this as a duplicate, feel free to reopen if not.

Hi @guillaumeg,

Sorry to hear it's still not working for you.

I've installed *just* the project_gtm module (added a GTM number), and added your patch, but it never steps into the
if (!empty($library[0]['#attributes']['src']) && $library[0]['#tag'] === 'script') { part of your patch.
Admittely they do add their script into the head, so maybe we do actually need to look at html_head for scripts that might get loaded.

In the mean time, I'm going to mark this issue as being fixed, but could i get you to please start a new issue with the reproduction and the patch (more preferrably an issue fork) so i can start work on your issue.

thanks!

Actually i think FileExists class not available in Drupal 10.3, causes compatibility issue 🐛 FileExists class not available in Drupal 10.3, causes compatibility issue Active actually ended up being a duplicate of this. Apologies for that.

If that's the case could you mark it as duplicate?

Hi @antonio.lepore,

Thanks for making that change. I've merged that into the main branch.

Hi @newaytech!

Glad to hear that's all working, i've merged the changes into the main branch, will release a new version shortly.

Thanks so much for yours and everyones help on this issue.

Hi - going to close this as being fixed, a lot more of the discussion was made in #3528439.
If it wasn't fixed, feel free to reopen.

Hi Alexander,

Apologies for that, lets try that again shall we? I have re-rolled the patch and tested it against the 1.26 release.

Should be all good to go now!

Thanks @newaytech for that

So i've removed the unverified script checks from hook_page_attachments() and left it in the _alter() hook instead, as you pointed out.

Hopefully this resolves it for everyone, if so i'll get a new release out.

Looks like saveData has support right back to 8.9.x
So you could just replace the 2 lines with the `< 10.3` and it should be fine.
oh - if you could also just remove my TODO comment that'd be cool too.

Hi - shouldn't be using MR161, that just disables the javascript.
I've attached a patch from my branch of 3527482-category-scripts-fix. If you could try that please.

Hi,

After implementing MR161, did you happent to resave the settings? this would write out a changed eu_cookie_compliance.script.js file with the updated unverified script list in it.

You're correct in saying that Alex's patch undoes the purpose of the unverified script variable, so we need to find a better solution.

My next question is (as i've seen this, but unsure if its a problem), could i get you to put a little debug into public://eu_cookie_compliance/eu_cookie_compliance.script.js to print the category variable (console.log(catagory)). This should be the same as your category machine name i believe. But it obviously has to match options.categoryName in the script otherwise it won't execute.

Thanks for your help in solving this issue.

Hi @newaytech,

I've linked the issue to another that is similar. If you could take a look and test that would be great thanks.

Hi @codebymikey,

I've pushed a change to a new issue fork here, could i get you to test it and let me know how it goes?

As much as i would like to change to what you have suggested, i can't do it just yet. I'd like to just get the module a bit more cleaned up first.
BUT! lets create a feature request, and get it done as i do think that's a good idea!

Thanks @techmantejas

i'll get onto review this shortly.

I was thinking we could merge all these into the gitlab-resolve branch, so that #3408084 would resolve?

Hi @codebymikey,

Can't delete that bit of code, thats responsible for weeding out any scripts that won't be loaded on the page.

I'm working on a solution at the js script end presently to fix it.

Hi there,
so i take it the gtag.js isn't being included into the page? The purpose of the unverified scripts is to make sure that the script that EUCC has been tasked to run will actually be on the page. So in some way the gtag.js script isn't being included on your page.

i'll install that module and see if i can come with the same issue.

atowl → created an issue.

atowl → created an issue.

atowl → created an issue.

atowl → created an issue.

Hi @deepakkm

I'm going to close this issue and raise another for each of the validation issues.
I think this is wise since the phpcs is going to be a big change, and i'd rather have that as a separate commit.

atowl → made their first commit to this issue’s fork.

Thanks @codebymikey,
I've merged that, it'll be in the next release shortly.

Thanks Alex for the re-roll, i've made some corrections since the setCookies wasn't working.

I removed the version option, as this seems deprecated in modern times, feel free to correct me.
When set in the configuration, the Secure flag will now be set in the cookie.

I'm just wondering if someone happens to untick the config option for Secure, should the cookie consent pop up again?

Also - haven't done anything about httpOnly, should we be checking headers? i'm not sure if we can.

i'll leave this in needs review, if the community could test, and i'll look at merging this for the next release.

Thanks!

atowl → made their first commit to this issue’s fork.

yesct → credited atowl → .

Hi @leslieg

I have chosen Access control, Administration tools, User engagement as the 3.

Will mark this as fixed for now, but if you need to please feel free to reopen it.

Hi @pianomansam,

So given that line is in the request of #3480626, we could close this issue and merge that instead?

I've applied your settings, and the accept and reject buttons seem to stay in the same place.

The only setting i couldn't apply was filter.format.ultra_html, instead i used filter.format.basic_html

Could that be your issue?

Ok thanks for that @trackleft, Will close this as a duplicate of #3467210

If i still experience it after the release of that then i'll raise another issue

@atowl has taken up maintainership of the module.

Looks good to me, will merge this in.

We've been using this patch for sometime with no issues.
Marking as reviewed and tested

Hi igorgoncalves,

That'd be great thanks!

Grant

Hi @szeidler,

I've made some changes based on my comments, if you could review and get back to me that would be good.
Biggest change is that i've refactored it into a module.

Thanks!

hi @pianomansam

How much of this change are we talking about undo'ing?

Merged, Thanks!

Hi - testing this, the accept button is always on the left for me?

Thanks!

Hi,

You've linked this to the 2.0.x version which hasn't been released at all.

Which version are you using?

atowl → made their first commit to this issue’s fork.

Hi @szeidler,

Apoligies in the lateness of reviewing this, getting onto this now.

While i've been thinking about this, i wondered about moving this into a seperate module that we can ship with the EUCC? So if they want to migrate to Klaro, then this can be enabled? My reasoning is that if the user is happy with the module currently, then this tab will always be in their configurational view, and maybe someone doesn't want that? Any thoughts?

In the mean time i'll review this.

Thanks, Think this was left over from when i was installing the environmental indicator, so i've removed it.

Rebased that change, there's a Php Cs warning? but i'm thinking its not my commit?

Hi Christian,

Tested this again, looks like it's fixed, marking the issue as such.

Thanks

Hi nikolino,

Marking this issue as closed, please feel free to re-open it if it's still an issue.

Thanks!

Hi @appleaday,

I'm marking this issue was postponed as it sounds like we need more information for this issue.

thanks!

Hi All,

Since there has been no movement on this issue in 4 years, i am going to mark this as closed [won't fix]

Please feel free to open this again if it still an issue

atowl → made their first commit to this issue’s fork.

Hi vo_dev,

Thanks for the patch, tried to apply this to 8.x-1.x, but wouldn't apply cleanly. The issue is mainly in the libraries.yml file. Since you are modifying the JS, you'll need to minimize it as well. If you haven't already, try the issue fork, much easier than patches.

I am also not entirely convinced that when pressing Reject All, should clear the "Checked and disabled" boxes, as these would be those options regarded as necessary for the site to operate?

Hi caldenjacobs,

I've pushed a change to remove the deprecation for the revision loading as you pointed out.

Think now we just need to get it reviewed.

atowl → made their first commit to this issue’s fork.

This looks good, Can we get a review?