Javascript error causes site to not load properly when using aggregation

Thanks for the patch. Does this also solve the issue if you set custom values for the three cookie states under Advanced?

I will see if I can move the initialization of those values inside the behavior. I don't think they have to live in the global space, so it should be possible to solve this issue. I'll mark this as "Needs work" for now and look into it one of the coming days.

@ron_s, is there any chance you can help set up a test site that demonstrates the problem, or send me a database or advagg generated js from such a site?

I tried to enable advagg and a few related modules on a basic Drupal 7 site and can't seem to reproduce the issue here. The variables are loaded at the right place, and the script still works.

I'm also seeing this with the latest version on Drupal 7. The site is not yet using the module, no advagg or similar is used. The site _is_ using jquery_update with version 1.10. Reverting to 1.4 (core) does not fix the error (and causes others in custom JS then of course). Neither does completely disabling jquery_update.

What I'm seeing is that in the aggregated version, eu_cookie_compliance.js is still in its own file and is loaded as the very first aggregated JS file. JQuery is then obviously not defined yet. Why that happens and only when aggregated I'm not sure yet.

I did test downgrading to 1.36 and 1.30 instead of .37, that didn't help either ( I didn't reinstall with those lower versions).

Ping me/contact me if you want to have a look at the site, but not sure if you can really see anything useful from the outside.

I'll start looking more closely into this in the coming days.

Yes, disabling aggregation does help, but I think the problem I have is different than yours, because I get a different error. Mine is that jQuery is undefined. Sorry for not reading it properly, just looked at the issue title.

Sadly I can confirm I'm having the same issue since the last update. Same as in the issue summary.
We're not using AdvAgg on the page, just Drupal Core Aggregation.

It doesn't work for me, even if I disable advanced aggregation:

Uncaught TypeError: $.cookie is not a function
    at Function.Drupal.eu_cookie_compliance.getCurrentStatus (eu_cookie_compliance.js?r6mxlz:527:11)
    at HTMLBodyElement.<anonymous> (eu_cookie_compliance.js?r6mxlz:25:35)
    at Function.each (jquery.min.js?v=1.10.2:4:5347)
    at e.fn.init.each (jquery.min.js?v=1.10.2:4:1999)
    at e.fn.init.$.fn.once (jquery.once.js?v=1.2:55:40)
    at Object.attach (eu_cookie_compliance.js?r6mxlz:22:75)
    at Object.<anonymous> (drupal.js?r6mxlz:112:12)
    at Function.each (jquery.min.js?v=1.10.2:4:5401)
    at Object.Drupal.attachBehaviors (drupal.js?r6mxlz:110:5)
    at HTMLDocument.<anonymous> (drupal.js?r6mxlz:610:10)

I'm using jquery update module 2.7 and jquery version 1.10

For us the error is "Uncaught TypeError: Cannot read properties of undefined (reading 'cookie_value_disagreed')" if the script is enabled to load in the head (which is required to block certain scripts).

Still unresolved. The only working fix is to downgrade to 7.x-1.36! So the issue must have been introduced in 7.x-1.37 and should thereby be solvable.

We do NOT use AdvAgg!

These are the many changes introduced with 7.x-1.37:
https://git.drupalcode.org/project/eu-cookie-compliance/-/compare/7.x-1....

Mainly the code in js/eu_cookie_compliance.js is broken due to the fact that if put in header
Drupal.settings.eu_cookie_compliance may be undefined if placed before Drupal.settings were initialized.

So the implemented fallbacks are not well implemented as they can't handle an undefined Drupal.settings.eu_cookie_compliance variable.

I'd suggest to first set the fallbacks in an array and then override them with the actual values, if the keys exist.
This whole part:

  var cookieValueDisagreed = (typeof Drupal.settings.eu_cookie_compliance.cookie_value_disagreed === 'undefined' || Drupal.settings.eu_cookie_compliance.cookie_value_disagreed === '') ? '0' : Drupal.settings.eu_cookie_compliance.cookie_value_disagreed;
  var cookieValueAgreedShowThankYou = (typeof Drupal.settings.eu_cookie_compliance.cookie_value_agreed_show_thank_you === 'undefined' || Drupal.settings.eu_cookie_compliance.cookie_value_agreed_show_thank_you === '') ? '1' : Drupal.settings.eu_cookie_compliance.cookie_value_agreed_show_thank_you;
  var cookieValueAgreed = (typeof Drupal.settings.eu_cookie_compliance.cookie_value_agreed === 'undefined' || Drupal.settings.eu_cookie_compliance.cookie_value_agreed === '') ? '2' : Drupal.settings.eu_cookie_compliance.cookie_value_agreed;

Must be refactored and tested with eucc placed in head setting.

I'll try to find time this weekend. Thanks for the updates.

Thank you very much @svenryen! :)

7.x-1.36 is fine for us, so we'll keep on using that.

Can any of you share what settings you're using for Advagg, and what sub-modules are enabled?

I tried to enable Advagg and set it to the recommended (optimized) settings, but I don't get any JavaScript errors.

For now, since I couldn't reproduce this with Advagg on a fairly basic site, as a last resort I'm excluding this module's script from Advagg aggregation.

Let me know how the patch in the branch helps, or please provide an archive with an installable site + database that I can use to reproduce.

https://git.drupalcode.org/project/eu-cookie-compliance/-/merge_requests...

@svenryen, did you see #15 and put the code into the header?

My personal guess is, that this is the root cause. Not AdvAgg.

I hear you, but there's also the variable euCookieComplianceBlockCookies, and some code between the lines 57 and 73, which could be affected by the same bug.

Since disabling advagg did solve the issue for @ron_s, I'm offering the patch in #20, which should solve the issue.

If you want to make a patch where the conflicting code is in the head tag, feel free to go ahead. We just need to find a way to test it afterwards, and make sure we move all conflicting code to the head.

I'm also wondering why $.cookie() was undefined for @guardiola86, though since disabling advagg didn't resolve that issue, I'd think that's a different bug, not related to advagg or this module.

I've looked at this again, and it feels safer to move the lines of concern inside the behavior rather than reverting to defaults.

I'll give one example. Let's say advagg rebuilt the aggregated files in such a way that the variables are not available when the lines are executed, and there are custom values set, you would store the wrong value in the cookie. Then if at a later rebuild by advagg you didn't run into the same situation, you would have the real values available to the script and you would potentially show the banner again, since the values set in config, which are now being used, don't match the wrong value in the cookie.

I think this could happen, but I'm not sure.

If I'm not able to reproduce, I'll include the patch from #20 in the next version, and keep this issue open so that we can find a permanent solution.

I've looked at this again, and it feels safer to move the lines of concern inside the behavior rather than reverting to defaults.

Perhaps you could create a further MR and show the proposed changes to ensure we understand it correctly?
I'm adressing the logical issues from #15.

Perhaps you could create a further MR and show the proposed changes to ensure we understand it correctly?

This doesn't have highest priority, but if a different patch is posted with a solution that has no side-effects, I'd definitely review it.

Here's another idea, maybe this will work.

We could try wrapping the script in:

(function ($, Drupal) {

})(jQuery, Drupal);

Shouldn't that make advagg respect that we need the Drupal variable for our command lines and place the code in such an order so that Drupal.settings.eu_cookie_compliance is available?

@Anybody and @ron_s, can you try the patch from the MR?

https://git.drupalcode.org/project/eu-cookie-compliance/-/merge_requests...

I've tested it here and moving the initialization of the variables inside a function seems to work well. Does it also resolve the issues you're seeing with advagg?

I'm going to take the risk of merging this code. There are no changes, just variable renaming done through Find/Replace (stepped through and validated every single change). I need to work on #3275786: 7.x: Banner at top is not hiding correctly which also changes a lot in the JavaScript.

I will also test the module extensively together with another maintainer before I tag a new release.