Closed (fixed)
Project:
EU Cookie Compliance (GDPR Compliance)
Version:
7.x-1.25
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
13 Jul 2018 at 18:05 UTC
Updated:
21 Dec 2018 at 18:59 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
svenryen commentedActually - the old "No I want to find out more" didn't block any cookies. It simply opens the privacy policy.
If you want a Disagree button, can I suggest the "Opt in" or "Opt out" methods?
I'm considering including a Legacy template for those who still want to use "Consent by default" and liked having the two buttons in the banner.
Comment #3
sachariew commentedYes, please add a template or another way to get the previous layout and functionality when we use consent by default. Thanks!
Comment #4
cobadger commentedI found this change to be abrupt, having just introduced this module to a couple of clients using consent by default setting.
@svenryen, patch attached with logic to use different info template for consent by default. Is this along the lines of what you had in mind?
Comment #5
svenryen commentedThanks for the patch. It would be great if it can be a choice, in case somebody wants to use the new template.
Comment #6
sachariew commentedHi, could we get something like this for D8 ? I am still at 8.x-1.0 because of this change.
Comment #7
cobadger commented@svenryen, to clarify: would this choice be for users who have chosen 'consent by default', allowing them to choose whether or not to use the new template?
Comment #8
svenryen commentedYes, correct. We need a setting that's only available for those who select 'consent by default'.
Perhaps a checkbox saying "Use legacy template for cookie information banner" at the bottom of the cookie information banner pane?
It should not be possible to select this option when you're using any of the other methods. Let's hide it in those cases.
Comment #9
cobadger commentedThanks for the clarification. New patch attached.
Comment #10
cobadger commentedRevised patch attached with correct logic for the config form's default value for the template selection form item.
Comment #11
adamps commentedSame here need the second button back to link to the privacy policy please.
@COBadger Thanks for the patch - it looks like a good start but I had some problems.
Comment #12
adamps commentedChanging category/priority as this seems likely to break a number of sites, so it would be great if we could get a commit and new version soon, many thanks.
I wonder, do we actually need a new setting? There is already an option to configure whether to show the second button, so can't we just stick with that? Whilst I agree that in default mode it doesn't make sense to have a "disagree" button, I certainly don't see it a 'legacy' variant to have a second button which could be labeled "more info" or "view policy" - the legislation seems to place a strong emphasis on communicating the policy to site visitors.
I think this affects D7 and D8.
Comment #13
cobadger commented@AdamPS thanks for your feedback on the earlier patch; updated patch attached.
I'm in favor of either keeping the legacy template as the only available template or at least allowing an admin user to choose it, as the structure of the new template moves a button into a different div and changes the styling.
Updated patch attached.
Comment #14
cobadger commentedI do not have a Drupal 8 patch for this issue. Changing status to 'needs review' for the Drupal 7 patch.
Comment #15
adamps commentedThanks @COBadger.
Apologies whilst rushing to apply security patches last week whilst travelling I had misunderstood. As per the issue summary, although the button is missing, there is a link instead. Hence resetting category/status.
Now that I've had a look at the new way, I've decided to stick with it, so I'm afraid I'm not going to be able to test the new patch. However I took a quick glance and it seems to be heading in the right direction.
Just a thought, but presumably anyone who wants the button back without using a patch can simply override eu-cookie-compliance-popup-info.tpl.php in the standard way. We could even attach the necessary template file and instructions here for the issue followers. It could save having yet another admin UI option when there are already many, and it would save the work still needed to create a D8 version of the patch.
Comment #16
svenryen commentedI think we need an option for this in the interface. There are still 17,105 sites at v1.14 and 20,411 at v1.23 who might at some point update the module, and I can see how they will be confused when the template changes.
If those who upgrade can get a "Legacy" template by default, we can at least prevent some people from panicking and opening issues, and those who just want to upgrade to get the security patch can do so without having to update their css.
The problem is most noticeable for those that applied custom styling to the "More info" button (some people misleadingly call it Disagree button) and that formatting spills over to the new link.
Comment #17
adamps commented@svenryen sure that's a good point and the current patch is getting close
But I think the patch might not do this part
Comment #18
cobadger commented@AdamPS / @svenryen revised patch, including db update to set 'legacy' template by default.
Comment #19
svenryen commented@AdamPS. I was going to suggest we sneak it into one of the existing update hooks. This way we won't revert the behavior for those who already upgraded, and those who upgrade in the future will get the legacy template applied.
This seems to be the least of all evils.
I can't really think of any other way that will work.
Comment #20
svenryen commentedThis update hook doesn't check which method the user currently is at. If they're using "Opt in" or "Opt out" it appears that their configuration will also revert to Legacy.
Comment #21
cobadger commentedThanks @svenryen for the feedback. We're not setting a variable to hold which template to use for the 'opt in' and 'opt out' options, only for the 'consent by default' option - so there's no risk that the wrong template will be used for 'opt in' or 'opt out' by setting this variable's value to 'legacy'.
Comment #22
cobadger commentedUpdated patch, moving this initialization into one of the existing db updates per @svenryen in https://www.drupal.org/project/eu_cookie_compliance/issues/2985662#comme...
Comment #23
svenryen commentedMissed that part (I hadn't read the entire patch in detail). Thanks for clarifying.
Comment #24
adamps commented#19 is very clever - I was about to comment again to point out that problem.
Comment #25
cobadger commentedUpdated patch to include a value for array item eu_cookie_compliance['info_template'] upon install, and to move a parenthesis in db update function.
Comment #26
cobadger commentedUpdated patch with correction to eu-cookie-compliance-popup-info-consent-default.tpl.php.
Comment #27
svenryen commentedThanks for the patch. It needs a port to Drupal 8.
Comment #28
summit commentedNice patch!
Comment #29
cobadger commentedPatch ported to Drupal 8 (attached.)
Comment #31
cobadger commentedTesting revised patch for D8.
Comment #32
cobadger commentedPlease review Drupal 8 patch:
eu_cookie_compliance-missing_button-2985662-drupal_8-31.patch
Comment #33
cobadger commentedApologies for all of the uploads.
Comment #35
cobadger commentedTestbot failed because there is no test coverage in the module.
Comment #36
svenryen commentedThe patch needed a bit of polishing.
CSS was slightly off, there were a few coding standard issues and a notice in Drupal 7.
Here are updated patches for D7 and D8.
Thanks for the hard work. This is an important issue.
Comment #37
svenryen commented