Problem/Motivation

Where the module is used cross-site with a sub-domain on another CMS such as WordPress, a 'consent given' cookie set by that site is not recognized. Although the cookie name is configurable, the cookie value is not.

Steps to reproduce

Proposed resolution

Make the 'consent given' cookie values configurable in the UI. Remove the check which restricts the cookie value to an integer value.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

John_B created an issue. See original summary.

john_b’s picture

Issue summary: View changes
svenryen’s picture

That's a valid point. I'll consider it, but it doesn't have top priority. Patches are welcome :)

svenryen’s picture

Priority: Normal » Major
svenryen’s picture

Title: allow cookie value to be set in UI » [2.x] allow cookie value to be set in UI

I'm flagging this for 2.x. If a patch arrives, we can also have this in 1.x.

svenryen’s picture

Version: 7.x-1.32 » 8.x-1.x-dev
john_b’s picture

I made a patch against the 7.x-1.x branch to allow configuration of cookie values for 'disagreed', 'agreed (show thank you banner)' and 'agreed'. Along with already configurable cookie name this allows ineroperability with similar modules on other sites. In our use case, we wanted it to be interoperable with the Complianz plugin for WordPreess.

We will be using D9 in due course, so probably will not work on a patch for 7.x-2.x.

john_b’s picture

john_b’s picture

There was an error in the above patch. Try again.

svenryen’s picture

Status: Active » Needs work

Could you please update your patch and remove the code for #3194891: background color and text color do not work, as it's seems that's not an issue with the module?

It's also not considered a good approach to mix code for two issues in the same patch unless the project members ask for it.

john_b’s picture

New version of patch without code for background and text color.

john_b’s picture

New version of patch, removing from js/eu_cookie_compliance_admin.js

  $(function () {
    showHideThankYouFields($('#edit-eu-cookie-compliance-popup-agreed-enabled').attr('checked', 'checked'));
  });

This code checks or unchecks the checkbox in the admin UI. The old code using the deprecated prop() does not whereas the version using attr() does.

svenryen’s picture

Status: Needs work » Needs review
svenryen’s picture

The patch seems to be for D7 so I ported it to D8.

Here's also an updated D7 patch that resolves some issues and adds an install hook.

@John_B are you able to review and test?

svenryen’s picture

Re-rolled patch for D8.

svenryen’s picture

New patch, fixing syntax switch from jquery.cookie to cookies-js.

svenryen’s picture

Fixed a bug that caused the banner to not show up.

john_b’s picture

Status: Needs review » Reviewed & tested by the community

This is largely based on my own work so I hesitate to mark it 'reviewed'. However, it has been working in production, so I will mark it 'RTBC'.

svenryen’s picture

Thanks, I'll merge later.

  • svenryen committed 9ef281c on 7.x-1.x
    Issue #3162563 by svenryen, John_B: [2.x] allow cookie value to be set...

  • svenryen committed bae71f1 on 8.x-1.x
    Issue #3162563 by svenryen, John_B: [2.x] allow cookie value to be set...
svenryen’s picture

Status: Reviewed & tested by the community » Fixed

  • svenryen committed 744b0d9 on 7.x-2.x
    Merge branch '7.x-1.x' into 7.x-2.x
    
    * 7.x-1.x:
      Add new file
      Issue #...
  • svenryen committed 9ef281c on 7.x-2.x
    Issue #3162563 by svenryen, John_B: [2.x] allow cookie value to be set...

Status: Fixed » Closed (fixed)

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

anybody’s picture