Closed (fixed)
Project:
EU Cookie Compliance (GDPR Compliance)
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
12 Jul 2018 at 16:09 UTC
Updated:
27 Aug 2022 at 10:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gabrimonfa commentedI would like withdrawing to be optional
Comment #3
svenryen commentedThe withdraw feature can be disabled. Simply visit the settings page, and there's a checkbox to toggle it on or off. I'm investigating an issue where it was enabled when it shouldn't be. This release was rushed out due to a security patch that had to be included.
Comment #4
svenryen commentedI like the idea, upunkt, and will see what I can come up with.
Comment #5
portulacaI'm also having trouble with styles. My banner is long (lots of text) so I had to use the height 100% and overflow: auto to make the banner scrollable.
But the changes apply to the withdraw popup as well, which then covers the entire viewport, making almost the entire site unusable.
It would be good to have the Withdraw link at least, so it can be placed in a menu, or something else that would make it easier for both users and admins.
Comment #6
jo.st commentedI had a similar issue in one of my projects.
The solution for me was to provide a menu item in the footer navigation, which gets created after both consent to tracking and denial. By clicking this button it is possible for users to change their cookie settings.
Seems to work for my projects (Google Analytics, opt-in).
This patch is by far not a complete solution. Maybe a starting point, hope it helps.
Comment #7
svenryen commentedComment #8
norman.lolFinally found out this can be done with only a little bit of JS. In the following sample I append a li element to the footer menu to withdraw/reaccept cookies on click.
Comment #9
svenryen commentedLike you mention in the following ToDo, this needs to be generalized. For example a field could accept a jquery selector where the menu item should be placed. I'm also wondering if the "button" tag is making this feature a bit biased. I, for one, do not typically wrap my menu items in a button tag.
This would also need to be ported to Drupal 8.
Comment #10
anybodyIn general this is a great idea for both D8 and D7. I'd suggest to combine this with a JS API to allow to call a function to show the withdraw banner etc.!
Furthermore I'd vote for a real menu item instead of Javascript code. The menu item could be added to the "Navigation" menu and moved to the right menu if needed. It should just call the JS API.
Comment #11
anybodySuggest to postpone this on #3130015: Write & document public JS API for actions & events?
Comment #12
anybody1. We don't need to postpone here.
Drupal.eu_cookie_compliance.toggleWithdrawBanner();works so far and can be used here.2. Here's the solution for a custom link, for example on the Privacy policy page:
For that to work you have to enable "Enable floating privacy settings tab and withdraw consent banner" in the settings.
Alternatively use https://www.drupal.org/project/euccx
@Maintainer: Could you perhaps review and add that information to the module page or a separate API documentation page? I guess it's widely required on privacy policy pages.
3. I'll work on a menu integration patch for cookie settings for this issue.
Comment #13
norman.lolWould be nice to have that button in a block provided by EU Cookie Compliance.
Comment #14
anybody@leymannx: Yes, good idea to also put that button in a block. I'll add that too.
Comment #15
jo.st commentedHi,
thank you for looking into this.
As for #9, yes I agree. I thought it was more accurate to use a button than a link considering accessibility/semantics. But I did not put too much effort in this, so I can't really tell the exact reason anymore. Reading articles like
https://css-tricks.com/a-complete-guide-to-links-and-buttons/
again, I think it was mainly: Do I navigate to another page (link) or not (button)?
Comment #16
anybodyHere we go with the Drupal 7 version!
Features:
Wasn't easy to find a good way to add a menu item with JavaScript.
Please review and remember to enable the "Enable floating privacy settings tab and withdraw consent banner" in the EUC settings!
Drupal 8 version following. Thanks to #1543750: Allow menu items without path that should work without the dummy callback... but let's see!
Comment #17
anybodyHere we go with the according Drupal 8 version. Same as with #16. Please review!
Comment #18
anybodyComment #19
anybodySorry #17 was the wrong file. Drupal 8 version attached.
Comment #20
bramvandenbulcke commentedThe current withdraw banner is indeed problematic:
So, on a recent project I implemented a solutions like in #8 (thanks, leymannx!) but with a single button on the cookie policy page.
But the proposed functionality would also be nice, of course!
Comment #21
anybody@bramvandenbulcke thank you very much, did you try and review patch #19?
Comment #22
bramvandenbulcke commented@Anybody, no, as stated, I went with the jQuery solution.
Comment #23
Anonymous (not verified) commentedThanks @Anybody, tested the patch #19. Added successfully menu item but didn't see block anywhere. Clicking menu item expands the settings tab. But actually that dont help much, since the tab is already visible (collapsed) and can be expanded anyway.
If floating Privacy Settings tab would be hidden after consent, and would reopen when clicking menu item, that would be nice solution.
Comment #24
anybodyHi @svenryen, could you please have a maintainer look at this? We're using this in production on several sites and it works very well. Drupal 7 and 8 version above.
Comment #25
svenryen commentedThanks for the patch. Any chance you can backport it to 7.x? (It probably can go in just for 8.x, but it would be nice to have it ported too).
Comment #26
anybodyHi @svenryen,
Patch for D7 can be found in #16,
Patch for D8 can be found in #19 :)
We're using both in several projects already.
Comment #27
robbm commented#19 appears to have applied successfully to 8.x-1.9 (via cweagans/composer-patches)m but doesn't entirely seem to have had the desired effect.
I can see the Cookie settings menu item suggestion in "Navigation" menu (on my "Tools" menu) and the Additional description on the settings page.
However, despite enabling "Enable floating privacy settings tab and withdraw consent banner", I can't see a Cookie settings block. (I assume that this should be available when I choose Place Block on the Block layout page?)
I've tried disable and re-enabling, clearing the cache, and running update.php. And there's nothing applicable in Recent log messages. Am I missing something?
Comment #28
anybodyHi @RobbM,
you're right, I will test the patches without "Enable floating privacy settings tab and withdraw consent banner" as soon as possible and ensure this works with all options. Thank you for reporting that. Meanwhile it would be nice to have more feedback with that option enabled.
Comment #29
anybodyAny plans for further review by maintainer and future progress?
Comment #30
svenryen commented@Anybody, I applied #2926798: Cookie Compliance as a block today, which also offers a block. In what way is this solution better/different?
Comment #31
grayle commentedI believe, ideally, this would be an extra option. You'd have "enable withdraw consent" and then choose "show floating tab" and/or "enable toggling popup using list of identifiers".
The new option would support a list of jQuery identifiers (class, id, whatever is valid) and the JS would simply listen for clicks on those elements to toggle the popup.
That seems the most extensible, possibly also least amount of work/future maintenance. People can give a normal link in a piece of text a certain class or id, enter the class/id in the settings form and clicking that link will a) preventDefault and b) toggleWithdrawBanner(). Or the same with a button, or a menu link or even an entire div.
Because if this gets in, the next request will be that they want to change something about the block or the link or whatever. Now, they make whatever they want and hook it up to toggleWithdrawBanner() in the settings.
We could go a bit better, but JS isn't my forte, and use events to trigger toggleWithdrawBanner(). People can trigger it based on whatever they want, they just have to fire the toggleWithdrawBanner event and this module will listen to it and toggle the banner.
Comment #32
svenryen commentedRe #29, it seems we are going to use this patch.
Re #28:
Can we agree that this patch needs more work, until it's been determined that it works with "all options"?
Comment #33
svenryen commentedComment #34
anybodyRe #32: Indeed, I'll do that ASAP, currently I'm really above my limits, too much work. I'll pick this up as soon as the situation improves. We'll do a sprint on the issues I'm participating in and 2.x but I can't yet say when the time has come... sorry. Just a human ;)
Comment #35
svenryen commentedComment #36
anybodyI now did a reroll here with the code from above and will test and improve it to work with "all options". See fork above:
7.x: https://git.drupalcode.org/project/eu-cookie-compliance/-/merge_requests/11
aka https://git.drupalcode.org/project/eu-cookie-compliance/-/merge_requests...
8.x: https://git.drupalcode.org/issue/eu_cookie_compliance-2985390/-/commit/d...
aka https://git.drupalcode.org/issue/eu_cookie_compliance-2985390/-/commit/d...
Comment #37
svenryen commentedSetting this to "Needs review" cc @Anybody
Comment #38
anybodyThank you! If anyone is interested, it would be good to get a feedback list what works well and what not ("all options") as this is currently limited as written above. So we definitely have to work on this, but at least patches apply now against latest dev versions.
The idea behind is to simply expose a public JS (API) function to call to show the withdraw banner on button click and for non devs to provide a menu link and block containing a button which calls this public js (API) function.
Comment #39
svenryen commentedOK, in that case I set it back to "Needs work" until you've sorted out the feedback.
Comment #40
anybody@svenryen, this is still on my plan, still too busy, but I'll definitely care for it!
Comment #41
anybodyComment #42
mrpauldriver commented@Anybody - I have installed the patch from #36 and find the same snags reported by @RobbM in #27
You asked for feedback ...
Like @RobbM in #27 - I can not find a block anywhere. This is a bug.
The menu suggestion is found in the Tools menu. This is usually the first thing I disable with a new installation. It would be good to see the menu item duplicated in the User menu where it would probably make more sense. Heck, why not have it (disabled) in all the frontend core menus and let the site builder choose what to enable or not?
In actual usage, when toggling the menu item, the patch does a good job of showing or hiding the banner. I like how this works, it is simple and familiar.
When interacting with the banner in this way, I don't believe there is any need to display the privacy settings tab. How to make it go away?
1) Retain the tab and the enable the new methods? Hide with css if not needed (prolly easiest for the x1 branch).
2) Provide another checkbox to use the new methods instead of a tab.
3) Provide new config group where all, some, or none of the different options can be selected. (perhaps best for x2 branch).
Comment #44
anybodyAdded a minor improvement (rel="nofollow") to 7.x. In 8.x this problem doesn't exist.
Comment #45
svenryen commentedComment #46
svenryen commented@Anybody, is this issue ready for review?
Comment #47
mrpauldriver commentedLast time I checked, no block had been created.
If there was a new patch I could review again, but I'm not up to speed on the new git way of doing things.
Comment #48
svenryen commented@MrPaulDriver - the code is now on Gitlab. You can see Anybody has made a Merge request against 7.x.
https://git.drupalcode.org/project/eu-cookie-compliance/-/merge_requests...
Comment #49
anybodySorry for my late reply. Still very busy, but things get better ... This is not yet ready, sorry. We're using it on many sites, but in a specific configuration. I'll still have to check all comments above to test and implement other cases.
The change above was to improve the link generated (rel=nofollow)
Comment #50
mrpauldriver commentedRef #48. Sorry but I do not have any experience of the 7 branch. All my sites are D8+
Comment #51
mrpauldriver commentedD8 patch from #36 no longer applies to the latest dev
Comment #52
mrpauldriver commentedI'd just like to say, that I'm using this patch in production and it's a killer feature. Brilliant.
It would be good to see it move forward.
Comment #53
anybodyThank you @MrPaulDriver, yes we're also using it in many Drupal 7 & 8 projects. Sadly I didn't find the time yet to finalize this. If someone can take over and finish the patches, you're very welcome! I think the finish line can't be far.
For Drupal 9 we're leaving the EUCC project, switching over to COOKiES for future projects as I wrote here: #3130662: Roadmap to 2.0.x release.
Comment #54
anybodyJust improved the Cookie Settings edit link in the Drupal 7 Version to also be a fragment / anchor #editCookieSettings instead of a JavaScript link.
IMPORTANT for Drupal 7 Patch: Everyone who used the patch URL directly and retrieves an auto-update so, please check the menu items after updating to the new patch from previous commit! Presumably the menu item will fall back to the default and can be found in "Navigation" again. I didn't find a good way to prevent us from this happening, but it's still a WIP patch, I'm sorry and hope you appreciate the optimization as now it's no "broken" fake-link anymore.
Comment #55
anybodyHere's the latest Drupal 7 version from MR 11 as patch file as of NOW. See notes in #54 if upgrading to this patch!
Comment #56
anybodyComment #57
svenryen commentedComment #58
svenryen commentedComment #59
flefle commentedPorted patch #19 into 8-1.19
Comment #60
svenryen commented@anybody, is the issue ready to review with the patches from #55 and #59?
Comment #61
anybody@svenryen, yes I think so. While it may not be perfect for all cases. You'll have to decide, if it's OK and an improvement from your perspective.
It's some time ago I wrote this and since that we're using it on many projects in production.
Comment #62
anybodyPS: Didn't look into #59 as there's no interdiff.
Comment #63
svenryen commentedI updated the patch for Drupal 7 to fix code style issues.
Comment #64
svenryen commentedInterdiff between #55 and #63.
Comment #67
svenryen commentedReviewed and tested. Thanks for the contribution! :)
Comment #70
tijsdeboeckAfter testing the 1.20 beta I noticed the following:
When I enable Enable floating privacy settings tab and withdraw consent banner, I cannot seem to find the Cookie settings block that should be available when "Cookie settings" button. I did clear the cache multiple times.
At first glance, I didn't see an issue with the code, but I'll try to look into the beta code in detail, or ask one of my colleagues tomorrow.
Comment #72
svenryen commented@tijsdeboeck, the annotation was missing. That was easy to resolve. You can try the 2985390-option-to-place-withdraw branch and test that it works.
Comment #73
svenryen commentedI also double checked just now that the block is there in Drupal 7.
Comment #76
tijsdeboeck@svenryen, just tested the branch on D9, and it does work now! I guess this can be marked fixed 🎉
Comment #77
tijsdeboeckComment #79
svenryen commentedAwesome! Thanks for the review!
Comment #80
portulacaIs the Withdraw Block feature in the 1.20-beta1 version?
I updated to that version and I'm not seeing any Block related features, on the module Settings page, nor on the Block layout page when I try to add a block to a region.
__________________
I have custom CSS applied to the banners so now a banner is displayed all the time, either the consent banner or the withdraw banner, and they block viewing the content.
I noticed on another site where I don't have the new beta version but also don't use custom CSS that there is a small Privacy... tab, I like that, although it's not a block I can have more control over. Do I need to disable my custom CSS and redo it to get the banners to hide behind the tiny tab?
_________________
I'm getting a white screen with only one line of text about an error after trying to save Settings and there aren't enough permissions on the the files/eu.... folder. The form saves the changes, I reload the Settings page and it's all working ok. But maybe this case could be handled better? At least mention file permissions in the error if WSOD can't be avoided.
I got into permissions trouble after using sudo when updating db on module update because of /tmp permissions.
Comment #81
svenryen commented@portulaca - we have fixed both the issues in the -dev branch (the missing block and the WSOD). Could you try that one instead? Let me know if you're unable to use the dev branch and I can tag a new beta build for you.
Comment #82
svenryen commented> Do I need to disable my custom CSS and redo it to get the banners to hide behind the tiny tab?
That depends. Are you on Drupal Slack? Maybe we can discuss your question there?
Comment #83
portulacaI was able to update to the dev version. For anyone uninitiated the command is:
composer require 'drupal/eu_cookie_compliance:dev-1.x'It's not 8.x-1x-dev to set the version.
I am getting the Block with the dev version, and the WSOD seems to be gone if I change permissions on the folder, although the environment isn't the same as when I was updating from a previous module version.
So far so good.
I do have the layout issue on the site where I used my own CSS to fix some issues (my banner text is long and it can be larger than viewport, which doesn't play well with display:fixed, come content is cut off). I'll let you know if I find a solution.
Comment #84
svenryen commentedReopening that one as we have a few issues, including the button not looking great in the footer. I'll provide details later.
Comment #85
portulacaRe: the button text color not contrasting with background color:
Currently, the Cookie settings Block HTML is using an anchor
a class="button"element as a click-to-slide-banner.button type="button"element is more correct semantically, since the purpose is to manipulate an on-page element with js, it's not leading to another page or scroll to an element.If you change the element the Bartik will show a good contrast of text and background even in the footer area.
I'm not saying that the change should be made to fix the color contrast bug, it's just a lucky coincidence that the bug is no longer a problem after the better HTML is used.
Comment #86
portulacaOr maybe it can be considered an anchor, no matter that the content is hidden at first, to visual users. Then make it show with the :focus, just like Skip links are used in many themes. No need for js that way.
Comment #87
svenryen commentedI missed the latest comments. I'll address them in -beta3.
Comment #88
svenryen commentedClassifying this as a bug report so that we don't lose track of the remaining bugs.
Comment #91
svenryen commentedI changed the link to a button, as per the feedback from @portulaca. Thanks all for contributing and bringing this feature to 1.20!