🇩🇪Germany @jurgenhaas

You don't need an update path. Gin 3 and 4 are identical. Just that gin 4 uses semantic versioning and will be the maintained version moving forward.

I don't think Klaro should be responsible to style for any number of themes. At most, it should provide an infrastructure so that themes can make the Klaro visuals look nice. Therefore, extra classes like in the MR should not be necessary, the currently active theme would style the available components and there shouldn't be any CSS that needs the context of different themes.

The only exception should be Claro, Gin and Olivero as they are Drupal Core or Drupal CMS defaults.

For those themes, I wonder if the Klaro CSS could just leverage the CSS variables to define the Klaro styling. I would assume, that those 3 themes use the same, or very similar variable names and hierarchies.

Giving authenticated users access to the dashboard might be an issue as this is an admin page, uses the admin theme, and not every authenticated user may have access to the admin theme. As a result, the dashboard may look awkward.

We discussed more fine-grained redirects depending upon role and/or permission before. But the dashboard module has the redirect hard-coded, and we can't make any changes, unless they agree to either remove that redirect or make it optional so that we could come up with smart redirects by turning the fixed redirect off.

The _io suffix for fullcalendar was chosen because of the external domain fullcalendar.io.

In general, I think _library is not bad. I had suggested _js before, but either way should work.

@cslevy I can't reproduce the problem. I followed your steps above, and the published checkbox shows below the body field.

Is there maybe something else altering your form?

However, I'm surprised why the checkbox is below the body and not in the sticky toolbar. @saschaeggi do you have any idea how that could be?

This code breaks on PHP 8.2 and Drupal 10.4 - the protected in the constructor needs to be removed.

@joelpittet we didn't plan for a patch release because it doesn't really matter if you have gin_toolbar 1.0 or 2.0 because they are currently identical. Moving forward, 2.0 will progress and by then, a patch release will have been available if not earlier.

That makes sense, even though I don't see why the IP address should eventually not be available. However, it doesn't hurt to have safer code ;-)

jurgenhaas → made their first commit to this issue’s fork.

@aron novak you're right that pipelines should be identical to the live system, that's our philosophy as well. And yet, we're looking to behave differently. With the dry-mode, the question remains where you draw the line. Doing everything except the ban is almost exactly the same as disabling the whisper handling. You can see that in \Drupal\crowdsec\Buffer::addWhisperSignal where the actual ban happens 10 lines later than the decision to return because of the disabled whisper. All the lines in-between are pure variable collection. And it should be the responsibility of this module to make sure that those work correctly. The live site tests should not rely on them, do they?

Thanks @gripphon for the extra details. We don't know for sure if it's caused by the same thing, but it could be.

I looked through the code changes between 2.0.8 and 2.0.9 and nothing comes to mind what could be causing this.

So, something that helps us to reproduce the problem on a fresh Drupal installation would be required.

All tasks completed, tests added and all green.

Looking good.

When getting around to this, please only require drupal/gin as Gin itself already contains the gint_toolbar dependency.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

This is looking good and tests are all green as well. Thank you @atul_ghate, I've merged that and will publish another release.

@adinac do you want to follow-up on this?

Well, I'd say DRD implements the best practice here and if you want to alter that, you could still implement the hook_requirements_alter in a custom module.

Right, if nothing else, here is what we could do with ECA:

Listen to the config save event and if it's about system.site, move on: Read the page.front value and verify if it's an alias. If so, find the canonical path for it and override the value in the config.

LMK if you want to have an ECA model for that.

saschaeggi → credited jurgenhaas → .

@cslevy thanks for contributing to Gin. As for this issue, I have two requirements and hope you can go with them:

• Please turn the patch into a feature branch with an MR, patches are not really used any longer.
• As for the issue, can you provide step-by-step instructions on how that scenario you're fixing here can be reproduced and what the real problem is that needs fixing?

The CDN route is not feasible as it would turn the privacy achievements upside down. But as discussed in the referenced issue, here is what I can do, if nobody objects:

• Creating general projects for each webform library on d.o to mirror the current version of each library
• Tagged them with the same release numbers as the current original library
• Provide a recipe for Drupal CMS that requires the webform module and all those libraries from d.o, i.e. from packagist.org at that point

With that approach, webform is installable and usable from within Drupal CMS (also from project browser on other Drupal sites) and still upgradeable with the regular tools that are used nowadays, without the need for any extra plugin.

This is great news, thanks @marcus_johansson

Next steps: I'll clean-up the MR when the next Klaro release tag will be available, hopefully later today, and then I write some tests. After that, it can be merged.

That could work, it should be a recipe rather than yet another module, though.

jurgenhaas → created an issue.

Still, I wonder how users who just work in the UI and install the webform module with project browser would ever get that up and running. It sounds like this isn't possible right now, or am I missing something.

As for requiring the libraries by the recipe in Drupal CMS, yes, that possible. The original intent of this issue has been to make this a webform improvement in general. But if that's not what the maintainers want, then @pameeela needs to make the call how Drupal CMS wants to deal with that.

This works like a charm. RTBC !!!

Please give the new MR a try.

@flocondetoile you're correct. I missed that as I had content moderation turned on where we don't have that checkbox at all. I'll propose a fix in a minute.

Ah, just tested a bit and it's not that simple because the XML structure changes completely. Each "participant" is treated like a model of its own, i.e. pretty much as if multiple ECA models were contained in a single one. That requires us to re-architect the \Drupal\eca_modeller_bpmn\ModellerBpmnBase class.

@mysdiir you can easily give this a try by deleting the following 2 lines from the bpmn_io.css file:

72: .djs-palette-entries .bpmn-icon-participant,
79: .djs-context-pad .bpmn-icon-participant,

Visually, that should already be all that's required. But we need to test of saving and executing models, where plugins are contained in such "participants" will still work as expected.

@phenaproxima sure enough. There will not only be a new tagged release but maybe even a stable release of Klaro not soon from now. But let's keep this MR open anyways until the AI team had a chance to give us feedback. I will then add our typical tests there too.

We probably don't even need the || ^3.0.x-dev part. I've tried in a project with the Klaro module dev release and required the dev release of the library in the project, which still works, even though the module requires ^3.0.

So, it would be sufficient to require ~3.0.0 as you described.

I've updated the MR and removed the temporary klaro config entities as they are now in Klaro's latest dev release.

Updating title.

This is ready for testing and review. Please note, the MR contains a couple of things that will be removed later on when everything is tagged in releases, especially the klaro config entities will be removed again later and the composer.json in the privacy recipe will be built back later as well.

But to give you an idea on the integration works and looks like, checkout the MR, run a composer update and then do a fresh site install. When you then apply the AI recipe and configure a provider, you will see how consent management for both the chatbot as well as alt text generation works.

I'm closing this as a duplicate of 📌 Create a consent modal for the AI Chatbot. Active as the AI/Privacy integration is handled together.

@jan kellermann does this require a rebase before it can be tested and reviewed?

This works as expected, great work!

I think it's probably best to merge this into 3.x-dev and then I would ask the AI team to give this a try with instructions over in the Drupal CMS issue queue.

I have updated the 3.0.x branch of the klaro_js library → which made the latest version available at https://packagist.org/packages/drupal/klaro_js#3.0.x-dev

What would be great, if the composer.json of the Klaro module could declare the constraint like this:

        "drupal/klaro_js": "^3.0 || ^3.0.x-dev"

That way, Drupal projects would load the latest 3.0 stable release, but if we add the dev requirement in a Drupal project for development, we could force the usage of the dev version without having to make any changes to the module ever.

I've tested this at https://git.drupalcode.org/project/eca/-/jobs/3746720 but maybe the related issue addresses this already and differently. Maybe this one could then be closed as a duplicate.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

This is being worked on at 📌 Add servives for AI features Active and I'll ping you here when it's ready for testing. Shouldn't be long, stay tuned.

This is being worked on at 📌 Add servives for AI features Active and I'll ping you here when it's ready for testing. Shouldn't be long, stay tuned.

This looks great. Just one question, but still RTBC otherwise:

The setting contextual_consent_only doesn't seem to be used anywhere. Is this baked into the upstream library? But even then, the value should at least be forwarded, or am I missing something?

When removing klaro as a function argument in line 1, this causes follow-up issues, e.g. in line 15, line 39 and below, where the klaro object is being referenced. I'm not sure how that can be resolved. Tha browser console prints error messages "klaro is not defined".

This also works like a charm.

I guess we can't combine this app/service with the other mastodon app? If not, no worries. It can go in as it is.

I've tested with the JS lib which is just about 25% of the original size, and everything is still working as before. Great work @jan kellermann, let's get this in as discussed.

Just an additional note: when I add the defer attribute like this:

  js:
    /libraries/klaro/dist/klaro-no-translations-no-css.js:
      minified: true
      preprocess: false
      attributes:
        defer: true

it does NOT work any longer. The browser console shows an error that klaro is undefined. So, we probably need to go without that extra step.

Thanks @jan kellermann for going through this with me on screen. After we've updated all components properly and confirmed the settings, this is working as expected and ready to go. Very cool :-)

I've now tested in the body field with full-html and I get this display:

There is no inline consent button. Is that by design?

When I go to the privacy settings and accept Mastodon, the embedded Toot is loaded after a page refresh, but not immediately.

We dont need a Klaro! service for this module because it imports serverside so the client has no contact to 3rd party.

I can't confirm. I see several requests going to social.bund.de from the browser. Example: GET https://social.bund.de/api/v1/accounts/109750405502438273/statuses?exclude_replies=true&exclude_reblogs=true. The module seems to deliver fed-embed HTML elements and their javascript is then loading that content.

As for the iframe test, I'm going to do that now.

This fixes the issue, perfect.

Note for others coming here: the browser cache was really hard to convince using the new webpack file. So it didn't seem to work initially, but after a force reload, it worked as expected.

Oh, wow, this looks impressive. I'll give it a try right away.

Not sure about a separate issue, that's more dependent on how you want to organise things.

As for this issue, I wonder if you could provide some testing instructions? I'm not sure how I should be testing this if not with the mastodon module. I guess, @gaurav.kapoor has done the same thing when testing, according to his comments above.

Works as expected. Maybe this needs a Drupal CMS Release tag, I will ping @pameeela about it.

RTBC+1 looking good to me too.

I can't get this to work. I'm using the mastodon module and configured 2 blocks, that look like this in the raw HTML:

<div id="block-olivero-mastodonembedpost" class="block block-mastodon block-mastodon-embed-post">
  
      <h2 class="block__title">Mastodon embed post</h2>
    
      <div class="block__content">
      <fed-embed data-post="https://social.bund.de/@Umweltbundesamt/113662513183074352"></fed-embed>
    </div>
  </div>



<div id="block-olivero-mastodonembedlatestposts" class="block block-mastodon block-mastodon-embed-latest-posts">
  
      <h2 class="block__title">Mastodon embed latest posts</h2>
    
      <div class="block__content">
      <fed-embed data-user="https://social.bund.de/@Umweltbundesamt"></fed-embed>
    </div>
  </div>

So, there is no iframe or anything. I guess there is some more configuration required?

The Google font issue is at 📌 Remove Google Fonts from deepChat.bundle.js Active

As far as Drupal CMS goes, we don't need the DeepChat to implement anything regarding consent management. The Drupal Klaro module already implemented everything needed, we're just testing it.

What's important though, the DeepChat client loads Google fonts, which needs to be avoided. The Klaro maintainers have already opened an issue about that.

Not sure how to get Cypress to bypass and/or pass the Captcha element?

As the purpose of the captcha is to prevent bypass, the user should just that. It should not be possible to submit the form.

In customer projects we also sometimes need to get the form submit. For that we disable the captcha temporarily.

That way we can test both.

Adding eca.model.*.yml to .gitignore makes sense.

But bpmn_io should be kept for end users. @pameeela was keen to have this available for end users so that we don't keep the fancy features in the dark but expose them to the site builder.

What we've implemented in the bpmn_io modeller is the so-called upstream conversion. So, without having the proprietary XML data available, we're now able to re-create that from downstream. So, the ECA models are still editable with BPMN.io without any further action required by the user.

This MR is now ready for review.

Now that the related issue got merged, I get started on this one.

And the good news on top: ECA 2.1.0 has just been released, as well as BPMN.io - so I can bump their constraints across Drupal CMS at the same time as well.

there have been test *.html pages in J /libraries with XSS issues.

That's true, but we could easily clean-up the mirrored libraries and remove those docs and example directories.

Also, another benefit is that webform will then be installable with project-browser without the user having to bother about anything.

I've fixed the changed label in the eca.eca.privacy_setting_link.yml file such that it also needs to be in eca.model.privacy_setting_link.yml and at the same time did another rebase.

As tests have been green already before, this seems ready to go.

There is no established pattern AFAIKT, we've used the suffix *_js in other places before.

Adding the dependencies to Drupal CMS' composer.json does work, too. I just wonder why other users of webform shouldn't benefit from this improvement as well. The fact that a dozen JS libraries get downloaded and stored in the libraries folder, doesn't sound too bad. At least compared to the gain factor in convenience.

@phenaproxima can this be merged? I'm asking because there are important issues for the Klaro maintainers who need to follow-up on this to provide privacy integration of the new chatbot: 📌 Create a consent modal for the AI Chatbot. Active and 📌 Handle the Alt Text Gen AI features in the terms for Drupal CMS. Active .

jurgenhaas → created an issue.

jurgenhaas → created an issue. See original summary → .

grienauer → credited jurgenhaas → .

Yeah, that's it :-) Thanks @christosgeorgiadis for finding this.

I'm moving this to the Drupal core toolbar issue queue.

Yes, agreed. While it is literally off-topic, it really makes sense to optimize this code altogether. I've looked through and I'm happy with it, so setting to RTBC.

@paul121 maybe we should also open a follow-up issue to optimize \Drupal\gin\GinContentFormHelper::isContentForm as we should probably cache the result of that fairly expensive method.

Fixed this in MR!323 and all tests are green again.

Problem was that our new feature to keep the Klaro app for remote videos always enabled prevented the recipe from disabling them. And because of that, the ECA model never enabled the menu link as it's supposed to do that only if the app got enabled after it was disabled.

I've fixed that such that the new feature to prevent disabling checks if there is at least one remote video and if not, it allows the app to be disabled.

Thanks for the clarification, so that would be separate modules then. You're correct, that would work. But I doubt this could be done in the short timeframe that's available in light of the Drupal CMS requirement.

@liam morland do you mean sub-modules or separate modules? For the former, this won't make any difference, because only the main module comes with a composer.json, and that requires everything that itself and its packaged sub-modules potentially need. Only if some modules were moved into stand-alone separate modules, the list of required libraries would be reduced.

@smustgrave it won't help. The problem is that composer can't load the required library without the declaration of an extra repository. And that needs to be done manually in the root composer.json file, i.e. that's manual work and therefore not a solution for Drupal CMS.

As composer already knows about the repository of packagist.org, we can easily make the libraries available there and every composer can require them without any extra or manual setup. In addition to that, this approach also solves the need that the libraries then get installed in the right directory as well, i.e. /web/libraries. Also without any extra configuration.

The namespace for such libraries will be drupal/NAMEOFLIB.

After that change, I think that the extra webform libraries composer.json file won't be required anymore.

OK, I'll create the MR as soon as 🐛 Privacy policy link goes to a 403 Active has landed. Otherwise, we may run into strange issues.

jurgenhaas → created an issue.

@phenaproxima In order to be prepared if @pameeela decides for that approach, I've added to the ECA model, that YouTube and Vimeo remain enabled once they have been enabled. If we will go into a different direction, we can quickly revert that commit. I just wanted to have everything in place so that it can go into RC2 when this gets tagged.

@vasike thanks for your suggestion and the reference to the other issue. I truly agree and mark this as fixed. Of course, if anyone disagrees, please re-open.

I think if they did, they would just uninstall Klaro altogether?

Probably yes, but there could be those who think that some configured privacy makes sense to them, but not the remote video part. But maybe I'm making up scenarios that will never happen.

While being on it, we managed here to reduce the overhead dramatically by only loading Klaro's javascript only when really necessary. At the same time, Jan Kellermann from Klaro found a way to reduce the size of the external jsvascript file from 208KB to 64KB uncompressed. This was possible by removing support for old browsers. I think, this is a fantastic achievement.

I can't disagree at all. And being a privacy advocate, this is music to my ears ;-) Let's see what @pameeela decides.

Good thinking, I considered that too but see 2 potential issues:

Automatic re-enabling: that could be problematic if a site owner decides not to bother about privacy and disabling the apps manually. We should not prevent them from doing this.

Re-enable when at least one remote video is present on the site: that's an extra DB query and we shouldn't do that if not absolutely necessary. However, that wouldn't happen too often, only if we detected the state change first. So, maybe this is not a blocker.

The first one, however, would be an issue I guess. The only way around that would be that the ECA model would have to be disabled first before they disable the Klaro app. Hard to explain to a user, but maybe worth it?

Thanks @pameeela for introducing this.

Happy to help out with this, if maintainers agree to proceed with this. Here is what we've done in other places like Fullcallendar → or Klaro → :

• Create a general project on d.o as a mirror of the remote library
• Add a composer.json to that repository, set the name to drupal/NAMEOFLIB and set the type to drupal-library, also add the installer-name in the extra section to the name of the directory into which this library needs to be installed
• Push that code and create a release, ideally with the same version number as the referencing upstream library

The packaging system from d.o will publish that project to Packagist and you can now add drupal/NAMEOFLIB to require section of the composer.json of this project. From now on, the library will be installed automatically without the user having to do anything manually.

Of course, this needs to be done for each external library and yes, there will be some overhead to keep the mirrors up-to-date. From experience, I can tell, this doesn't happen too often and should be manageable.

Thanks @pameeela for introducing this.

Happy to help out with this, if maintainers agree to proceed with this. Here is what we've done in other places like Fullcallendar → or Klaro → :

• Create a general project on d.o as a mirror of the remote library
• Add a composer.json to that repository, set the name to drupal/NAMEOFLIB and set the type to drupal-library, also add the installer-name in the extra section to the name of the directory into which this library needs to be installed
• Push that code and create a release, ideally with the same version number as the referencing upstream library

The packaging system from d.o will publish that project to Packagist and you can now add drupal/NAMEOFLIB to require section of the composer.json of this project. From now on, the library will be installed automatically without the user having to do anything manually.

Thank you for your help on this @phenaproxima, this is really great.

Just one ting that came to mind: as the remove_video recipe disables the Klaro apps for YouTube and Vimeo, there might be a theoretical risk that those get disabled again, if somebody re-applies that recipe some time after they had already created some remote video media entities. Turning the apps off at that point is not what should be happening. But I don't see how we could prevent that edge-case.