Account created on 26 September 2008, about 16 years ago
  • Senior Developer / CTO at werk21 
#

Merge Requests

More

Recent comments

🇩🇪Germany jan kellermann

Thank you all for testing and feedback. I merge this and will tag a new RC.

🇩🇪Germany jan kellermann

Thank you, @van.dordafog
I changed the code and updated the MR.

🇩🇪Germany jan kellermann

This is function from Klaro! JS not the Drupal module.

But I found this:

If config-item "config.showDescriptionEmptyStore" is set to true, this text incl. link to the consentmanager will be shown: "To agree to this service permanently, you must accept {title} in the {link}." (descriptionEmptyStore).

If you like this please convert this Issue in en feature request :)

🇩🇪Germany jan kellermann

I also added also an "automatic" mode that open the notice only if neccessary. See screenshot.

🇩🇪Germany jan kellermann

You need to consent the use of klaro first - without this consent we can not store "always" because no cookie exists :)
Thats why I added an "auto"-mode in #3487461

🇩🇪Germany jan kellermann

I used with and without prefix. But you are right: checking the route is the better solution. Commit added.

🇩🇪Germany jan kellermann

@valthebald and @longwave are right, that this is not a violation of the GDPR - but of the EU ePrivacy Directive and national laws in european countries (since 2002!).

The ePrivacy Directive Art. 5 (3) says, that you need users’ consent before you "store information" (cookies, localStorage etc.) "in the terminal equipment of a subscriber or user" (e.g. browser) except "strictly necessary" data. This EU directive is not a direct law, but is implemented by national laws, in Germany the TDDDG and in Spain the LSSI for example.

The new EU e-Privacy Regulation is currently being drafted; this will replace the directive and will then become direct law (in the same way as the GDPR). According to the current status, it contains an analogous passage, see Article 8 "Protection of information stored in and related to end-users’ terminal equipment":

It is not about which data is processed, but about the protection of the visitor's end device - so it does not matter whether it is a tracking, 1st or 3rd party cookie.

It is a violation in all european countries.

🇩🇪Germany jan kellermann

Tested with current dev version. Thank you @van.dordafog!

🇩🇪Germany jan kellermann

Merged to dev. Thank you all for feedback and testing.

🇩🇪Germany jan kellermann

@sascha_meissner: The encoding back was missing at all :) I added this in last commit:

diff --git a/src/Utility/KlaroHelper.php b/src/Utility/KlaroHelper.php
index 7c54f58..336d418 100644
--- a/src/Utility/KlaroHelper.php
+++ b/src/Utility/KlaroHelper.php
@@ -805,6 +805,7 @@ class KlaroHelper {

     if ($complete_html) {
       $html = $dom->saveHTML();
+      $html = mb_decode_numericentity($html, [0x80, 0x10FFFF, 0, 0x1FFFFF], 'UTF-8');
     }
     else {
       $html = Html::serialize($dom);
🇩🇪Germany jan kellermann

Its because of the (new) icons in the comments in twig debug…

🇩🇪Germany jan kellermann

Thank for your comment. I added the test.

Please review.

🇩🇪Germany jan kellermann

Can you please double check if twig debug is enabled (see #3483397)?
Else please test https://git.drupalcode.org/project/klaro/-/merge_requests/14/diffs

Thank you for reporting.

🇩🇪Germany jan kellermann

Yes, I learned, too. Now we are using a json variable instead of inline-JS:


    $page['#attached']['html_head'][] = [
+      [
+        '#tag' => 'script',
+        '#attributes' => [
+          'type' => 'application/json',
+          'id' => 'gin-darkmode',
+        ],
+        '#value' => new FormattableMarkup('{ "ginDarkmode": "@value" }', ['@value' => $settings->get('enable_darkmode')]),
+      ],
+      'gin_darkmode',
+    ];

And my CSP does not report anymore.

🇩🇪Germany jan kellermann

You are right, the question is if the storage item is technically necessary.

> I can follow the argument that it is technically necessary because there is no other way to follow the user's preference.

The user has not made any preference at this point. Only the website operator's default setting is saved in the visitor's browser. That is the problem, that it is NOT a setting made by the user. Hence my approach of working without storage until the user makes a selection.

🇩🇪Germany jan kellermann

You didn't understand the judgment. It is not about the type of cookie, but about the fact that data was stored on the user's device without consent. See also Article 22.2 LSSI.

The ePrivacy Directive says, that you need users’ consent before you use any cookies except strictly necessary cookies. This EU directive is not a direct law, but is implemented by national laws, in Germany the TDDDG and in Spain the LSSI.

The new EU e-Privacy Directive is currently being drafted; this will replace the directive and will then become direct law in the same way as the GDPR. According to the current status, it contains an analogous passage, see Article 8 "Protection of information stored in and related to end-users’ terminal equipment".

It is not about which data is processed, but about the protection of the visitor's end device - so it does not matter whether it is a tracking, 1st or 3rd party cookie.

🇩🇪Germany jan kellermann

Thank you for feedback. I changed this issue to a feature request.

We will have a look if we can support blazy.

🇩🇪Germany jan kellermann

Let's try to find the problem.

1) Are all src-attributes correct changed to data-src?

Please check if in your markup and look in your browser's developer console if no data is loaded from vimeo.

2) Did you test in a new private tab in your browser?

Often old cookies exists for your domain or subdomain, so please open a new private tab and delete cookies.

3) Which entity / field do you use?

Can you please post the config (like core.entity_view_display.media.embed.default.yml)?

🇩🇪Germany jan kellermann

Changes from new dev-version on generel settings page.

🇩🇪Germany jan kellermann

Fixed field labels.

🇩🇪Germany jan kellermann

Added screenshot of the settings form.

🇩🇪Germany jan kellermann

Added consent for tracking as objective.

🇩🇪Germany jan kellermann

Thanks to @eyedrops for committing the first tests for Klaro!

🇩🇪Germany jan kellermann

I just pushed a candidate for testing and discuss.

All cookies and storage items will be blocked until the consent for the service was given.

Two apps are "required" per default:

  • Klaro with cookie '^klaro'
  • Drupal with cookie '^[SESS|SSESS]'

A new service is published per default:

  • CMS Convenience with cookie '^Drupal\.'

All localStorage and sessionStorage e.g. from Drupal (Claro theme or Gin theme) will be blocked until the consent for "CMS Convenience" is given.

All blocked data is written to a local data structure and written to the cookie storage or localStorage / sessionStorage after consent has been given. In this way, no data is lost during blocking.

After revoking consent, not only the cookies will be deleted from Klaro-JS but also the sessionStorage and localStorage.

Please have a look and give feedback.

🇩🇪Germany jan kellermann

After merging the dev branch the install-file was broken.

I changed the settings form according to the discussion in #3485880.

I also changed the JS commands for silent mode. In my tests everything was fine.

Please review (hopefully for the last time).

🇩🇪Germany jan kellermann

Thank you very much!

I fixed the missing margin and will merge this MR.

Ich changed the wording because the close-button will also added to the non-modal notice dialog.

Lets discuss the settings form in #3484938

🇩🇪Germany jan kellermann

Thank you got the exact and extensive feedback! It's great to have you on this project.

I worked on CSS and JS to fix all the focus bugs and put the elements in the right order.

I also worked on the settings form and introduced a new section "Required consent" and I refer in the description to the button section.

Please reviews.

🇩🇪Germany jan kellermann

I have made the code and the example a bit more general. But we should avoid exposing fields if they are not needed. More field preprocessors can be proposed as a feature request.

🇩🇪Germany jan kellermann

Ic created a Merge request. Beside the code I have added example code to the README.md

What do you think, should we add the data attributes by ourselves (by option) for Media Remote Video or just provide the sample code?

🇩🇪Germany jan kellermann

Thank you, @rkoller for your time! I am sorry, that you could not test :(

The new option should be in the button area, see screenshot.

Maybe the patch did not applied? The patch is against klaro-dev and not latest rc.

Maybe you can checkout branch `3485880-make-close-button` from Issue-git ?

Thank you very much.

🇩🇪Germany jan kellermann

Added :)

Please review and please check the accessibility of the new close button.

🇩🇪Germany jan kellermann

The close button is only added for notice dialog, not for consent dialog modal. I will add this.

🇩🇪Germany jan kellermann

I added a close button X to the notice dialog. You have to enable the button on the settings page.

Please review.

🇩🇪Germany jan kellermann

Thank you for your feedback!

Here is the upstream issue: https://github.com/klaro-org/klaro-js/issues/478

We will add a button via Drupal until klaro upstream offers this function.

🇩🇪Germany jan kellermann

Here is an actual example why storing data in user‘s browser needs consent:
https://gdprhub.eu/index.php?title=AEPD_(Spain)_-_PS/00524/2023&mtc=today

🇩🇪Germany jan kellermann

Thank you larowlan, But the white page persists.

We changes serialize() with json_encode() and removed the first unset() befor the return statement to prevent recursion in different trees of array.

And for Drupal we had to remove the patches for test.

🇩🇪Germany jan kellermann

We had this problem, too. The update of twig/twig was triggered by roave/security-advisories:dev-latest.

We hat to downgrade and fix twig/twig to 3.14.0 in composer.json

🇩🇪Germany jan kellermann

@grienauer Thank you for your comment! For the gin login page you need a consent in the moment - but I hope #3475773 🐛 Setting localstorage without consent on login page Postponed: needs info will be fixed soon ;-)

In general: There are other consent options for logged-in users. For websites with registration, you can add a checkbox to the registration form to indicate that the backend stores data in the browser or uses social media. And do employees have to consent if the data processor is their company itself and the browser is on their working laptop?

So I dont think that you need klaro in the backend in most cases, this can be solved with the editorial team in a different way.

You can set the permission to access the UI for anoymous users only. Or we can add an option “dont show an admin pages” (please open ticket).

There is also ticket #3484938, with which the notice box can only be used if it is needed.

🇩🇪Germany jan kellermann

I just added some options and color as "low hanging fruits".

Further MRs are appreciated.

State unchanged.

🇩🇪Germany jan kellermann

I added config and an update-hook. Please have a look.

The new javascript is quite little tested - we need more than one review if possible (also with ajax and big pipe).

🇩🇪Germany jan kellermann

Image as link

🇩🇪Germany jan kellermann

I added the preprocess for field formatter html from module html_field_formatter.

I added also inactive services for:

  • Bluesky
  • Facebook
  • Instagram
  • Linkedin
  • Mastodon (as example)
  • Threads
  • Tiktok
  • X

Please review.

🇩🇪Germany jan kellermann

You are completely right! The old wording of Klaro was "App". Nowadays all people are using "service" for this. I will change this in backend, too.

🇩🇪Germany jan kellermann

@jurgenhaas maybe checkout and test. This branch overrides hard the setup of klaro (we should add some config for this).

- All external media will be protected.
- All external sources are not loaded (because no consents given).
- I think the small privacy button should stay (can later be disabled, i added in an other ticket a description for creating menu item).

Please have a look.

🇩🇪Germany jan kellermann

Thank you for the extensive feedback!

We are working with the upstream CSS from klaro. Maybe the better way is to open an issue on Github?

But of course we can add more CSS to our module. Maybe we can add an own Drupal Style and let the site builder choose between different styles (e.g. Klaro Vanilla and Claro Drupal")?

🇩🇪Germany jan kellermann

wow, thank you @rkoller!

> By placing the block in a layout region on the Block Layout page, the button is moved further down the DOM within the body element, making it potentially more challenging to find?

Yes, you are right. But the user has the requirement to place the markup elsewhere in the DOM. The advantages are not clear to me personally either. Maybe it's about the semantic order in the DOM.

> my expectation would be if no block is being added, there wouldn’t be any consent modal visible.

Our aim is to ensure that Klaro can be used with as few settings as possible. The block function is just an additional function for repositioning in the DOM. We have written this again explicitly in the help text.

> The button is the first and the div for the consent dialog is the last child of the body element. The unexpected detail the title of the block is not visible anymore, and there is also no empty element for the block in the secondary menu region.

Could not reproduce. Maybe a caching problem?

> And one question if the Klaro block, like in this case, is added to the secondary menu region, is it correct that the wrapping element for that block is a nav element?

This is a special feature of this region in Olivero theme.

> And the micro copy of the choose klaro elements for this block is sort of unclear and redundant.

See new version please.

> I suppose all those details in the context of Olivero and the social bar are out of the scope for this issue and are probably better suited for a follow up issue?

Yes, I added a note for adding CSS code.

> I would only recommend adding an aria-haspopup=“dialog” attribute to the button

Thanks for the suggestion and the separate issue. I reviewed an merged already.

> I would recommend wrapping the consent dialog into a dialog element instead of a div with the role of dialog.

I am glad that you are contributing your detailed knowledge! You can open an issue here or we can do this - as you like: https://github.com/klaro-org/klaro-js/issues

Thank you at all for your long feedback. As a result, we agree that automatic placement is the best option and that placement in blocks only makes sense in exceptional cases. I have tried to clarify this again in the help text,

🇩🇪Germany jan kellermann

MR checked out an the aria-attribute appears in the markup.

Production build 0.71.5 2024