Montpellier, France
Account created on 16 September 2010, about 14 years ago
#

Merge Requests

More

Recent comments

🇫🇷France DuaelFr Montpellier, France

duaelfr created an issue.

🇫🇷France DuaelFr Montpellier, France

Hi there! Thank you for reaching me.
I don't remember having been asked to name someone as maintainer of this module before so I apologize if I forgot or missed something.
This is now done. Thank you for stepping up and helping to make Drupal better.

🇫🇷France DuaelFr Montpellier, France

@anybody You must be right about getEntity() as the \Drupal\Core\Entity\EntityViewBuilder::viewField() method calls \Drupal\Core\Entity\EntityRepositoryInterface::getTranslationFromContext() right after. The thing is that it does the call only for untranslatable fields and without any langcode so it fallbacks on the content language which is not the one we expect in most technical cases.
In my case, the field is not translatable so it wouldn't work anyway… I'll give a try to the #7 patch even if it looks a but hackish.

🇫🇷France DuaelFr Montpellier, France

Same issue here using a list_string type field with translated labels supposed to be used by pathauto to generate the path of the node.
That part of the path is always in the default language.

I traced that down to the $entity->$field_name->view($display_options); call in _field_tokens() where I found something really strange happening (which seem to be a Core issue).

$entity->$field_name->language()->getId() returns the appropriate language.
$entity->$field_name->getEntity()->language()->getId() (first thing done by the \Drupal\Core\Entity\EntityViewBuilder::viewField() method) returns the default language!

I didn't find anything related to that issue in the issue queue and I'm not sure I know how to file that. Any help would be appreciated.

🇫🇷France DuaelFr Montpellier, France

As you can see in !22, the fix is quite simple and I believe it would also fix the original issue explained in the IS.

🇫🇷France DuaelFr Montpellier, France

I faced this issue too but a bit different as it was not language related but user related.
I think I found steps to reproduce and the underlying cause of this.

Steps to reproduce

  1. Create three pages: /test1, /test2 and /test3 (paths are important)
  2. Add two pages to the main menu: [root] > test2 > test3 (test1 is not in the menu)
  3. Clear all caches
  4. Access /test1 - expected breadcrumb: [Home]
  5. Access /test2 - expected breadcrumb: [Home] > test2
  6. Access /test3 - expected breadcrumb: [Home] > test2 > test3

Current results:

  • Breadcrumb for /test1: [Home]
  • Breadcrumb for /test2: [Home]
  • Breadcrumb for /test3: [Home]

Note: [Home] only shows if breadcrumbs are configured that way. If they are not, the breadcrumb just disappear.

Why is that?

This seems to be related to the way Core caches its own breadcrumbs. The way Core builds its breadcrumbs only needs to know the parent path of the current page. That's why they declare url.path.parent as a cache context and not url.path like Menu Breadcrumb does. What happens is that if you first load a page where the breadcrumb is not managed by Menu Breadcrumb, then the breadcrumb block will be held in cache for every other pages sharing the same parent path. While using url.path.parent is a nice optimization in the Core's breadcrumbs management, it's what's causing our issue here.

🇫🇷France DuaelFr Montpellier, France

Thank you for opening and working on this issue.
I have a use case where translations are automatically generated by TMGMT but as the module clones both the orignal node and its translations, these do not trigger the TMGMT job as it considers the entity as already translated.

Having a global configuration like suggested above seem to be a good way to solve my issue.
Patch #22 is working for me and the code looks harmless for existing projects so it shouldn't need any upgrade path.

I think #24 has a point too but that it's a bit out of scope here. Maybe it should be addressed in another issue.

🇫🇷France DuaelFr Montpellier, France

Rerolled on the latest 11.x
+ added the ability to configure link options translatability (might be used by contrib like Link Target )

🇫🇷France DuaelFr Montpellier, France

I can confirm patch from #12 is working well. Thanks!
It's a bit strange to have merged this issue with 📌 Support content language in the selection mode Needs review but as I needed both I can't complain too much ;)
The maintainer might want you to split your patches to make it easier to review, though.

🇫🇷France DuaelFr Montpellier, France

Wow! It sounds very promising! Looking forward to this feature!

🇫🇷France DuaelFr Montpellier, France

Rerolled on latest dev version.

🇫🇷France DuaelFr Montpellier, France

I rerolled the MR on the latest dev version.

🇫🇷France DuaelFr Montpellier, France

Rerolled MR on latest dev version.

🇫🇷France DuaelFr Montpellier, France

My bad, patch doesn't apply cleanly on 6.0.x (we need 3-way merge).
Here is the one for those in need for this right now.

🇫🇷France DuaelFr Montpellier, France

The patch can be committed to 6.0.x as well.

🇫🇷France DuaelFr Montpellier, France

FYI phpunit failure on the MR seems unrelated

🇫🇷France DuaelFr Montpellier, France

@queenvictoria Thank you for letting me know! You're right, I did a mistake rerolling this and making the patch. Here is a new one.

🇫🇷France DuaelFr Montpellier, France

Rerolled !3037 MR against 11.x
Patch attached for people needing this on 10.3.1

🇫🇷France DuaelFr Montpellier, France

@besek MR's patch is against 3.x-dev so it does not apply on rc11. This patch is a workaround for people who need this change but cannot wait for the next release and/or cannot switch to the development version of the theme.

🇫🇷France DuaelFr Montpellier, France

Here is the patch for people wanting to apply it directly on RC11.
I can confirm it fixes the issue for the media edit modal introduced by the Core patch here Allow media items to be edited in a modal when using the field widget Postponed

🇫🇷France DuaelFr Montpellier, France

Updated IS to get more feedback about #8's proposal.

🇫🇷France DuaelFr Montpellier, France

Patch from #8 is working and looking good.
It's better than what's suggested in #6 because getTranslationFromContext allows fallback languages to be selected which can be very useful for some projects.

MR rebased!

🇫🇷France DuaelFr Montpellier, France

DuaelFr made their first commit to this issue’s fork.

🇫🇷France DuaelFr Montpellier, France

@Berdir #15 > The language module provides *only* the entity type + bundle-level setting, that scales fairly well. Only when you also enable content_translation then you get the vast list of per-field-per-bundle checkboxes. Those two things were added at different times, and both pretty early in the D8 lifecycle.

What if we only changed the bit added by content_translation to only add links/buttons in this form leading to a specific page for each entity type? I think we don't even need to split it to a page per bundle to begin with (could be done in a follow up).

🇫🇷France DuaelFr Montpellier, France

Good job here!
One concern though:
Isn't "Add" an inappropriate button label in some rare edge case where CSS is disabled or overridden somehow? I believe it would be better to use another string explaining what's happening here like "Loading" for example.

🇫🇷France DuaelFr Montpellier, France

No need to keep that open :)
Thanks Dave!

🇫🇷France DuaelFr Montpellier, France

DuaelFr changed the visibility of the branch 3172550-register-drupals-mime to hidden.

🇫🇷France DuaelFr Montpellier, France

Given #22 and #26 I rerolled the tests to see what the testbot has to say about this old issue.

🇫🇷France DuaelFr Montpellier, France

I traced down the CDATA removal to \Drupal\Core\EventSubscriber\RssResponseRelativeUrlFilter::transformRootRelativeUrlsToAbsolute()

I updated MR !7201 with a fix.

🇫🇷France DuaelFr Montpellier, France

I did the exact same change in my own views-view-row-rss.html.twig for both the title and the description.
While there is no issue with the title, the description gets filtered somehow.

I tried to duplicate the description field with another name:

<item>
  <title><![CDATA[{{ title }}]]></title>
  <link>{{ link }}</link>
  <description><![CDATA[{{ description }}]]></description>
  <anything_else_but_description><![CDATA[{{ description }}]]></anything_else_but_description>
  {% for item in item_elements -%}
    <{{ item.key }}{{ item.attributes -}}
    {% if item.value -%}
      >{{ item.value }}</{{ item.key }}>
    {% else -%}
      {{ ' />' }}
    {% endif %}
  {%- endfor %}
</item>

The "anything_else_but_description" field does not get filtered out.
Does anyone know where it could happen?

🇫🇷France DuaelFr Montpellier, France

Patch is really simple and causes no harm :)

🇫🇷France DuaelFr Montpellier, France

Yes! I can confirm it is still an issue. See \Drupal\filter\Plugin\Filter\FilterCaption::process().
I cannot find any restriction on the CKEditor side anymore, though.

Backend:

Frontend:

🇫🇷France DuaelFr Montpellier, France

Sounds legit, code is straightforward, tests are green and paragraphs are saved as expected.
Thank you!

🇫🇷France DuaelFr Montpellier, France

I needed that so I rerolled #14 into a MR.
For those who need it right now and don't want to jump on the dev release, here is a patch against the 1.15 version.

🇫🇷France DuaelFr Montpellier, France

DuaelFr made their first commit to this issue’s fork.

🇫🇷France DuaelFr Montpellier, France

This has been committed so the issue should be marked as "Fixed".
I faced a new issue so I opened a follow-up 🐛 Warnings when using this on HTML5 markup Active to continue improving the module. We might want to write tests at some point.

@mlncn I don't have much time to spend on maintainership but I can jump in if you need.

Documentation improvements might be discussed in another issue: 📌 Improve documentation Active

🇫🇷France DuaelFr Montpellier, France

DuaelFr created an issue.

🇫🇷France DuaelFr Montpellier, France

Hi @mkalkbrenner and thanks for your work here!
I was trying to solve this using a dead simple trick that was to use env vars to set the core name in the settings.local.php.
I was hoping to be able to run a drush sapi-i call with the env var but as this rely on the search_api_item database table, I wasn't able to do that that simply.
Would you provide some guidance either to find a workaround or implement that swapping feature in the module, please? I'm willing to provide a patch but I'm not sure I wouldn't loose myself without at least a starting point.
Thanks again!

🇫🇷France DuaelFr Montpellier, France

I have no idea why it's happening but I'm facing the same issue with my simple date field in a facet widget.

    $form['date_start'] = [
      '#type' => 'date',
      '#title' => $this->t('From', [], ['context' => 'date_range_widget']),
      '#default_value' => $existingValues['min'] ?? NULL,
    ];

I can only fix the issue by removing the type attribute added in both \Drupal\Core\Render\Element\Date::getInfo() and \Drupal\Core\Render\Element\Date::preRenderDate().
Interestingly, this does not prevent the actual tag in the markup to have the appropriate type attribute.

🇫🇷France DuaelFr Montpellier, France

Rerolled MR. Still needs tests.

🇫🇷France DuaelFr Montpellier, France

Rerolled !9 MR on HEAD

🇫🇷France DuaelFr Montpellier, France

@stBorchert Yep, no problem for me! Thank you for asking :)

🇫🇷France DuaelFr Montpellier, France

Fixed stupid mistake in the MR (I wasn't using the forged html string in the loadHTML method...)

🇫🇷France DuaelFr Montpellier, France

I can confirm that the proposed patch/MR fixes the issue.
I agree with @ckrina that this is not the ideal solution, though. I wonder if it wouldn't be better to create a new library for vertical-tabs styling and make both vertical tabs and media library libraries depend on it. Any thoughts, boss? :)

🇫🇷France DuaelFr Montpellier, France

For people having issues after upgrading to Drupal 10.2 with vertical tabs having lost their styles: 🐛 [Drupal 10.2 regression] Media Library "widget" View media type tabs have lost styling RTBC

🇫🇷France DuaelFr Montpellier, France

I just opened the !1 MR with option 3 from my previous comment.

🇫🇷France DuaelFr Montpellier, France

Hi! Thank you for this module!

Sadly, this change breaks utf8 support.
For example: <p>légende</p> is converted to l&Atilde;&copy;gende which is then displayed as légende.

Looking for a fix, I've come to 3 options:

  1. use utf8_decode() on the string passed to the loadHTML() method
    how:
      $dom = new DOMDocument;
      $dom->loadHTML(utf8_decode($text));
    

    pros: one line fix
    cons: might mess with some specific characters (not sure), possible issue if the source string is not using utf8 (is it possible in Drupal?)

  2. use the loadXML() method instead of the loadHTML() one
    how:
      $dom = new DOMDocument;
      $dom->loadXML($text);
    

    pros: one line fix
    cons: could break if the given HTML is not perfect (ie: unclosed tag), could be mitigated by running this filter after the filter_htmlcorrector filter from core but that would be in the site builder hands

  3. encapsulate the string into a minimal HTML structure before passing it to the loadHTML() method
    how:
      $dom = new DOMDocument;
      $charset = mb_detect_encoding($text);
      $html = "<!DOCTYPE html><html><head><meta charset='$charset'></head><body>$text</body></html>";
      $dom->loadHTML($html);
    

    pros: workaround the cons of other options
    cons: looks a bit hackish

🇫🇷France DuaelFr Montpellier, France

I rerolled MR !3037 on 11.x based on the code from #134 and #141 (next patches were missing some code).

The attached patch applies on Drupal 10.2

🇫🇷France DuaelFr Montpellier, France

Patch from #142 was not applying because it has been created inside the core folder instead of at the root of the project.
This patch solves that issue and applies on 11.x (and 10.2)

🇫🇷France DuaelFr Montpellier, France

Quick and dirty patch & MR for the bonus option as I need it right now ;)

🇫🇷France DuaelFr Montpellier, France

I opened an upstream issue: https://github.com/ckeditor/ckeditor5/issues/15715

The suggested fix inspired by #11 could be added to our admin theme CSS as a workaround.

.ck-text-fragment-language-dropdown .ck-dropdown__panel {
  overflow-x: auto;
  max-height: var(--ck-dialog-max-height);
}
🇫🇷France DuaelFr Montpellier, France

Hi! FWIW when I first tried to fix that issue, I realized that the buttons behavior was different given the allowed advanced settings selected in the filter. Take that into account when testing your changes because it could work for your specific case but break another one… I'll be happy to merge a change working for all use cases!

🇫🇷France DuaelFr Montpellier, France

DuaelFr created an issue.

🇫🇷France DuaelFr Montpellier, France

Thank you all for your work on this one!
Sorry for the huge delay :/

🇫🇷France DuaelFr Montpellier, France

Fixed in 1.x and cherry picked in 2.x.
File usage is tracked for nodes by default so files uploaded in webforms or custom forms should be tracked differently.

🇫🇷France DuaelFr Montpellier, France

DuaelFr made their first commit to this issue’s fork.

🇫🇷France DuaelFr Montpellier, France

PHP Coding Standards do not apply on JS files. We have specific coding standards for JavaScript and we use ESLint to check them.

🇫🇷France DuaelFr Montpellier, France

Thank you all for the hard work here! <3

🇫🇷France DuaelFr Montpellier, France

#86: yes we have to update existing templates or the issue won't be fixed. Adding the tag.

🇫🇷France DuaelFr Montpellier, France

Thank you for your involvement.
You're now a maintainer of this module! Congratulations!

🇫🇷France DuaelFr Montpellier, France

Sounds good. Thanks!

🇫🇷France DuaelFr Montpellier, France

DuaelFr made their first commit to this issue’s fork.

Production build 0.71.5 2024