Marking this issue critical as it currently results in a WSOD.

Updated to 8.x-1.1 and this issue started, updated to 8.x-1.2 and this issue persists.

The website encountered an unexpected error. Please try again later.Symfony\Component\Debug\Exception\ContextErrorException: Notice: Undefined variable: primary_button_label in eu_cookie_compliance_page_attachments() (line 182 of modules/contrib/eu_cookie_compliance/eu_cookie_compliance.module).

Drupal\Core\Render\MainContent\HtmlRenderer->invokePageAttachmentHooks(Array) (Line: 273)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 76)
Drupal\webprofiler\EventDispatcher\TraceableEventDispatcher->dispatch('kernel.view', Object) (Line: 156)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 38)
Drupal\webprofiler\StackMiddleware\WebprofilerMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 666)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Turned out that in eu_cookie_compliance.module line 134 $method = $config->get('method'); returned NULL and therefore the following code was breaking.

Replacing it by $method = $config->get('method') ? $config->get('method') : 'default'; fixed it. Probably it's just the config logic that has changed. Probably just need to be able to login again and reexport config to ultimately fix it.

Patch following.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

leymannx created an issue. See original summary.

svenryen’s picture

Did you run drush updb? Do you have translations on your site?

leymannx’s picture

Status: Active » Needs review
FileSize
560 bytes
leymannx’s picture

Yes, I did run drush updb. Issue persisted. Yes, I do have translations on my site.

leymannx’s picture

Yup, what I assumed. On an existing site, with the new changes, Consent method has never been exported and therefore the code breaks. As I see now inside config/install/eu_cookie_compliance.settings.yml the default is supposed to be opt_in. Updated patch accordingly.

Spokje’s picture

Having the same issue, but with different symptoms.

Site with Drupal 8.5.3 and translations, eu_cookie_compliance 8.x-1.2.
Did the DB-update, seeing the Notice in the log, but no WSOD.
However the text on the Agree Button is empty.

Patch #3 fixed it for me (since site was using "Consent by default").

svenryen’s picture

Thanks for updating the patch. I'll have a look this weekend.

leymannx’s picture

After thinking this over, the patch from #3 indeed seems to be the better choice. As it applies the default method which has been the default prior to version 8.x-1.1 and therefore should be the method that is chosen for everybody who had the module already up and running before the 8.x-1.1 release.

Unfortunately also the order of elements in the markup in the main templates changed. Would be nice this also would have been switched only after a method via config had been set explicitly. Good news is we have identified a marker to distinguish between older and newer installations, which could be used to provide some fallbacks to prevent things from breaking after the feature-rich 8.x-1.1 release.

Anyways, besides these minor flaws thanks for this great module and the ongoing development, I really appreciate that! :)

Spokje’s picture

Status: Needs review » Reviewed & tested by the community

I agree with @leymannx that patch #3 respects the previous settings the closest. Happy to put it on RTBC.

I also agree with @leymannx that the work on this module is greatly appreciated, and I really like the new options.
Having said that, in hindsight it might not been the best idea to roll those, with so many changes, into a Security Update.
An "in-between" Security Release might have gone down easier.

svenryen’s picture

The problem was that a security hole was found and reported in the -dev branch, and it was difficult to coordinate a security update without at the same time releasing the dev branch.

Spokje’s picture

@svenryen: Ah, that makes sense.
Thank you for clarifying :)

marcoliver’s picture

Any idea when this might make it into a release?

svenryen’s picture

Within a month is the current estimate.

svenryen’s picture

It's been exactly a month and I've been busy. I'll commit it to dev later and hopefully get a new release out when some bug reports are settled.

  • svenryen committed 673ed64 on 8.x-1.x authored by leymannx
    Issue #2985543 by leymannx: Notice: Undefined variable:...
svenryen’s picture

Updated D7 to match the default from this patch. The check was already there.

  • svenryen committed 8aaff9f on 7.x-1.x
    Issue #2985543 by leymannx, svenryen: Notice: Undefined variable:...
svenryen’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.