Problem/Motivation

After reinstalling the custom profile with the EU Cookie Compliance module installed, the user settings are reset to default

Steps to reproduce

  • Install EU Cookie Compliance module
  • Change some text settings:
    • Popup agree
    • Popup info
    • Mobile popup info
    • Withdraw message
  • Exports configs to your custom profile
  • Reinstall profile
  • Find out that the data was not saved
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Alexander Kalashnik created an issue. See original summary.

Alexander Kalashnik’s picture

Alexander Kalashnik’s picture

Status: Active » Needs review
Alexander Kalashnik’s picture

Status: Needs review » Needs work

The last submitted patch, 4: issue_3189864_4.patch, failed testing. View results

Alexander Kalashnik’s picture

Status: Needs work » Needs review
Phil Wolstenholme’s picture

Title: Settings from configs are not imported » Install hook rewrites message text without checking if the message has been changed, preventing sites installed from a profile from customising the message text
Related issues: +#2979440: EU Cookie Compliance expects specific text formats to exist

I had this issue too, I've updated the title to be a bit more specific.

The issue is caused by these lines: https://git.drupalcode.org/project/eu-cookie-compliance/-/blob/8.x-1.x/e...

They reset the values of thee popup_agreed, popup_info, mobile_popup_info, withdraw_message messages and text format. This would be fine for a fresh installation of the module on a site that has not used it before, but it causes issues when the site is being reinstalled using a profile that already contains customised message/format settings in a eu_cookie_compliance.settings.yml file within the profile.

Alexander's patch works for me, now when I reinstall my sites using a profile the text format and message settings are no longer wiped by the install hook.

A next step is to check that the patch from this issue still fixes the original issue described at https://www.drupal.org/project/eu_cookie_compliance/issues/2979440.

bwaindwain’s picture

Patch #4 works for our distro. Thanks @alexander-kalashnik!

codebymikey’s picture

I believe the original #2979440: EU Cookie Compliance expects specific text formats to exist issue has a point, and the module should validate whether the format exists.

However the current #2979440 code is still fundamentally broken, as it doesn't work on "standard" profiles either because at the point where eu_cookie_compliance is being installed, there's no reference to the restricted_html format, so it always defaults to plain_text.

With the current patch, the config will always exist, so the ternaries in the current patch might be unneeded - the #2979440 code can be simply removed in order to achieve the original behaviour.

I've added a patch which allows individual sites to opt out of the behaviour using a settings.php override:

$settings['eu_cookie_compliance_validate_filter_format'] = FALSE;

It can be argued that the default setting value should be switched around, as minimal profiles are probably in the minority, and should be the ones to opt in to the new behaviour instead, but as the code is already committed and released, it's currently an opt-out to avoid introducing yet another breaking change.

Alternative solutions are to:

  1. Remove "eu_cookie_compliance" from the profile's dependencies and install it programmatically - however it means the module may be uninstalled, which might break certain functionality.
  2. Provide an install task which reapplies the appropriate eu_cookie_compliance.settings profile configuration post install (slightly better).

svenryen’s picture

Status: Needs work » Needs review

  • svenryen committed 62485fd on 8.x-1.x
    Issue #3189864 by Alexander Kalashnik, codebymikey, svenryen, bwaindwain...
svenryen’s picture

Status: Needs review » Fixed

I changed the cookie banner message, exported config, then installed using existing config and the changes were preserved.

Thanks for the patch!

Status: Fixed » Closed (fixed)

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

zacharie.montreuil’s picture

I've noticed that the patch no longer works in 9.2.x -- a straightforward reroll will use the proper message settings, but not the text format.