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

Merge Requests

More

Recent comments

🇩🇪Germany jan kellermann

I have now made the necessary adjustments.

Please review.

🇩🇪Germany jan kellermann

I changed the test like suggested. Thank you for your helping hints.

🇩🇪Germany jan kellermann

Yes, you are right. During our research, we did not find this module and therefore developed this module and also AI-provider-WEB-T on the basis of the existing WEB-T module. We use the settings and the translation service of the WEB-T module. We also use the data handling of the WEB-T module, which, among other things, ensures that only the translatable parts of an HTML text are transferred and translated.

🇩🇪Germany jan kellermann

Can you please post link to the change record? I didnt find. I only saw some issues for assistants. AnythingLLM is not using agents (see #3480528 🐛 PHP Notice if no Actions configured in AI Assistant Active ).

Thank you for reporting?

🇩🇪Germany jan kellermann

Reviewed and works for me. Solution also seems appropriate.

🇩🇪Germany jan kellermann

Thank your for discussing.

Do you have some additions for the README or documentation?

Else I would like to close this issue.

🇩🇪Germany jan kellermann

Thank you for reporting! I just added a small comment to your commit.

Can you please explain when this bug occurs and how to reproduce?

🇩🇪Germany jan kellermann

Thank you for reporting. We are not native speakers, so we welcome any hints.

I think we can remove “be” completely from this sentence. What do you think?

🇩🇪Germany jan kellermann

The original issue is #2409107 🐛 Do not prepopulate the user register form with the email address and username of the last person who registered using the same web browser Needs work (and is celebrating its 10th birthday soon). In this comment 🐛 Do not prepopulate the user register form with the email address and username of the last person who registered using the same web browser Needs work some module are linked which are using this functionality.

So I opened a ticket to find a fast lane to remove this in core, see #3498834 🐛 Dont use core's prepopulate function for core forms (Privacy) Active . Maybe we can quick bring this ticket to an end and then remove this from core general with deprecation in this ticket.

🇩🇪Germany jan kellermann

I just adapted the core from core to extract the embedded media enities. These can be embedded in every text field, not only the in the node, but also in referenced paragraphs, for example. So we should process every text for referenced medie entities.

But what to do then?

The existing program code is based on existing fields. It seems difficult to add further content here; it may be necessary to add pseudo-fields that are added before the translation and processed separately later.

Another approach could be to create separate jobs on the fly for the connected media.

I welcome suggestions from maintainers and can then try to provide more code.

Here is the pure function for extracting:


use Drupal\Core\Entity\EntityRepositoryInterface;


  /**
   * Process text for embedded media entities.
   *
   * @param string $text
   *   The string to translate.
   *
   * @return array
   *   Array of embedded media entities.
   */
  public function extractEmbedMedia($text) {
    if (stristr($text, '<drupal-media') === FALSE) {
      return [];
    }

    $embedded_media = [];

    $dom = Html::load($text);
    $xpath = new \DOMXPath($dom);

    foreach ($xpath->query('//drupal-media[@data-entity-type="media" and normalize-space(@data-entity-uuid)!=""]') as $node) {
      /** @var \DOMElement $node */
      $uuid = $node->getAttribute('data-entity-uuid');
      $media = $this->entityRepository->loadEntityByUuid('media', $uuid);
      $embedded_media[] = $media;
    }

    return $embedded_media;
  }

See core/modules/media/src/Plugin/Filter/MediaEmbed.php

🇩🇪Germany jan kellermann

Danke Dir vielmals! Hab noch kleine Anpassungen vorgeschlagen.

🇩🇪Germany jan kellermann

As you see on the module page, the module is replaced by Google Tags - thats why we did not write a Service Config for this module.

You can find supported modules in our README:
https://git.drupalcode.org/project/klaro#services-for-google-tag-manager...

Please ask if you need futher support.

🇩🇪Germany jan kellermann

There are some modules for GA. Can you please post the link to the module you are using?

🇩🇪Germany jan kellermann

There a two large translation systems: TMGMT and AI.

TMGMT

For TMGMT we are working on the plugin module tmgmt_webt. We wrote our own service for API-requests. But I would prefer to have one centralized service from webt. If you make auto-translation optional or move to a sub-module we can go this way.

AI

At the moment the ai_translate only uses chat-based translations (see #3477180). The module ai_provider_webt is ready written using webt-service. Here we need your decision if you want to decouple service and auto-translation.

Technically it is only a config item and adding a if-clause in this line. We would write an update hook to enable per default for legacy.

🇩🇪Germany jan kellermann

Merged. Thank you all for feedback and distributing.

🇩🇪Germany jan kellermann

I reverted the last commit and changed the first commit to place the close-button at the end of dialog and not footer. You should place your footer now sticky and the close-button should remain top right in the dialog.

Please review and feedback.

🇩🇪Germany jan kellermann

Thank you for feedback, rkoller. We have the Klaro! button "Manage consents" which already has the attribute aria-haspopup="dialog". I would also add this attribute to the "customize"-button. I would also add type="button".

I am not able to add any attribute when the consent-dialog is open - because the notice is removed from DOM.

Please feedback and I will start.

🇩🇪Germany jan kellermann

As I understood it, the AI-Translate module is supposed to deliver a provider for “translate_text” that works with LLMs and chat.

My impression is that AI-Translate works past “translate_text”. I cannot select the set Deepl provider at any point.

Was this the goal? What is the sense of “translate_text” then? How can I use ai_provider_deepl with AI-Translation module?

🇩🇪Germany jan kellermann

Because the given project already used this field type.

A solution based on remote video / oembed would be nice either.

🇩🇪Germany jan kellermann

Every Drupal has its own entities. In webt.module you are testing for instanceof ContentEntityBase and that is a good way.

Many modules are creating their own entities. There are config entities also which may need translation.

🇩🇪Germany jan kellermann

In preparation of this ticket i found this function in src/TranslationManager.php:

  public function getTranslatableFields($entity) {
    switch ($entity->getEntityTypeId()) {
      case 'node':
        return [
          'title'       => ['value'],
          'body'        => ['value', 'summary'],
          'field_image' => ['alt'],
        ];

      case 'comment':
        return [
          'subject'      => ['value'],
          'comment_body' => ['value'],
        ];

      case 'taxonomy_term':
        return ['name' => ['value']];

      default:
        // Type not implemented.
        return [];
    }
  }

You hardcoded the translatable fields. But what to do with all the other fields? And you can not presume that these fields really exists. This should be replaced by a field iterator per entity.

🇩🇪Germany jan kellermann

Thank you for feedback! I changed the MR like suggested.

Some question (a little offtopic):

You hardcode the bundles:
```
$this->checkContentTypeTranslationEnabled(StringType::NODE, [
'article',
'page',
]);
$this->checkContentTypeTranslationEnabled(StringType::COMMENT, ['comment']);
$this->checkContentTypeTranslationEnabled(StringType::TAG, ['tags']);
```
Wouldnt it be better to get all bundles per content type?

🇩🇪Germany jan kellermann

Thank your for discussing.

Do you have some additions for the README or documentation?

Else I would like to close this issue.

🇩🇪Germany jan kellermann

We discussed the close button for some weeks in https://www.drupal.org/project/klaro/issues/3485880 Make close button X as DeclineAll button for mustConsent: true (needed for Italy) Active
Please have a look. I cannot remember all the problems we discussed.

The behavior to "discard all" is only needed, if no consents are saved (in notice dialog and in consent dialog). The consent dialog has its own close-button - except if option "must consent" is enabled, then Klaro hides its own close button. So we added a close button with "discard all" function for this both cases. In all other cases the close-button is like an "abort"-button.

🇩🇪Germany jan kellermann

> Not sure what these conditions are meant for, but removing them fixes the problem. In that case, everything works as expected.

The conditions control when the Drupal module has to add its own close button. Else the close button fom Klaro JS is used.

🇩🇪Germany jan kellermann

The 2nd markup is from Klaro JS.
The 1st markup is provided by Drupal module.

So we cant fix the Klaro JS markup :(

The Klaro JS only shows close button on some situations. You may open an upstream issue on github.

🇩🇪Germany jan kellermann

I created a MR. Please review and feedback.

🇩🇪Germany jan kellermann

Works for me. But the order of interactive elements changes!
Maybe move the new element to the end of .cm-modal.cm-klaro'?

@rkoller can you please have a look for a11y?

🇩🇪Germany jan kellermann

We can add the aria-expanded="false" via JS in Drupal. But we cannot change this state because after clicking the whole markup is removed from DOM. Does it still make sense to use this aria attribute?

🇩🇪Germany jan kellermann

Thank you, niklan! You are of course absolutely right that we need an update hook for the addition of config entities and that the schema also needs to be updated. (I had understood that we would split the existing scripts into the new areas - which I think is too risky.)

🇩🇪Germany jan kellermann

I think we cannot write an update hook for old javascript snippets. We have to use both variants in parallel and deprecate the old way in next major release.

🇩🇪Germany jan kellermann

The GDPR uses the term “service”, which is why we do the same.

The term “Klaro app” was previously used, but has now been replaced by the standardized term “service”.

See also this issue: https://www.drupal.org/project/de/issues/3484856 📌 Klaro Active

🇩🇪Germany jan kellermann

I think that in principle of “privacy first” this should be decided by the data controller,

You could include a passage in the documentation.

I will publish the new version now. :)

Thank you for your work.

🇩🇪Germany jan kellermann

I checked plyr player module and you can use it with simple config items. I added an example config.yml.

However, I would recommend the plyr module maintainer to load the sources locally. This would also avoid the setTimeout().

🇩🇪Germany jan kellermann

Big thanks to kensae and ckng. I have to check in the next days and will add some tests for the new options.

🇩🇪Germany jan kellermann

Hi! Big thank for your contribution! I merged an we can release the next minor release whenever you are ready.

Small notice: you may enable "optOut" it the tool is really GDPR-compliant (but optOut is not working because of a bug in upsream klaro-js).

🇩🇪Germany jan kellermann

Added new Hooks Implementation. Please review and feedback.

🇩🇪Germany jan kellermann

I added base_path and tested succuessfully.

Please review and feedback.

🇩🇪Germany jan kellermann

I have added another patch to allow an empty prefix so that the behavior is unchanged if the prefix is not set.
If a preset is set, it should also be applied. I doubt the sense of an update script to rename the keys, as it can lead to even more errors with shared hosting. A change request notice should be created here.

🇩🇪Germany jan kellermann

I diffed both versions:

3166c3166
<                 const a = e.getConsent(s.name) && e.confirmed
---
>                 const a = e.getConsent(s.name) && (e.confirmed || s.optOut)

This is the expected change, so the new version should be correct.

🇩🇪Germany jan kellermann

Thank you very much for this bug report.

Bug is fixed, please review and feedback.

🇩🇪Germany jan kellermann

Thank you for issue, feedback and testing.

I merged this MR.

🇩🇪Germany jan kellermann

Works for me!

Thank you very much for contributing!

🇩🇪Germany jan kellermann

I just the changed the form field description (you cannot "uncheck" a text input field).

But I would prefer an option to hide "Powered by" message because on multilangual sites you have to delete for every language.

🇩🇪Germany jan kellermann

This is standard behavior of klaro. We will add an option to remove this message. See #3491582

🇩🇪Germany jan kellermann

There is an answer: https://github.com/klaro-org/klaro-js/issues/526

  • You can remove the "Powered by Klaro" message from the UI.
  • You must keep the BSD-3 license text and attribution in the source code.
  • Providing recognition elsewhere (e.g., documentation, config page) is a good practice but not legally required in the UI.

So we can add an option under styling.

🇩🇪Germany jan kellermann

Wow, good work!

But why the module owners do not use the Drupal-way of placing their javascripts? Then you could just some values for "Attachments" and you are ready.

Maybe you can ask them to work with Drupal standard. Else you can open a MR with your code as sub module which can be enabled if dfp is used.

Thank you again for your work with this module.

🇩🇪Germany jan kellermann

It is not possible to give a order of purposes for Klaro! JS. The order cannot be guaranteed by a different sequence of services, as a service can have several purposes.

We have to add the order of purposes to DrupalSettings and order the purposes by javascript.

🇩🇪Germany jan kellermann

Bump. Updated fork, ready to review. Would be great to have this issue reviewed and merged for next release.

🇩🇪Germany jan kellermann

Bump. Updated fork, ready to review. Would be great to have this issue reviewed and merged for next release.

🇩🇪Germany jan kellermann

Merged. Thank you for testing and feedback.

🇩🇪Germany jan kellermann

Thank you for feedback and testing.

I have added the information to the readme file.

I don't expect any security implications if we don't check whether the module is installed. If someone has access to the code and can manipulate field values, they can also inject code elsewhere. We just do the rendering and let the Klaro helper check the result.

So I set the state to RTBC.

🇩🇪Germany jan kellermann

Thank you for testing and feedback!
I merged the MR and will release soon a new version.

🇩🇪Germany jan kellermann

I added applyConsents() while initialization.

Please review and feedback.

Production build 0.71.5 2024