Problem/Motivation
Very similar to the original post. Layout builder add custom blocks were not displaying the block types in the translated language.
Original Post
In Layout Builder, when you go to add a block to a section, and click "Create custom block", a list of links of the type of custom block content you want to add as a inline block appears. On a multilingual site, this list of links displays only in one language, which is language the site is using when it caches the derivate plugin definitions.
This is because of line 52 of docroot/core/modules/layout_builder/src/Plugin/Derivative/InlineBlockDeriver.php:
$this->derivatives[$id]['admin_label'] = $type->label();
Which sets the admin_label of the derivate plugin definition to a string instead of TranslatableMarkup.
| Comment | File | Size | Author |
|---|---|---|---|
| #38 | 3038717-38.patch | 20.59 KB | tonibarbera |
| #32 | 3038717-nr-bot.txt | 144 bytes | needs-review-queue-bot |
| #28 | interdiff-26-28.txt | 1.06 KB | smustgrave |
| #6 | 3038717-derivatives-5.patch | 2.52 KB | tim.plunkett |
Issue fork drupal-3038717
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
tim.plunkettThis is visible using Block UI as well, and not just with custom blocks.
Menus and Views are also affected.
As well as field blocks within Layout Builder.
Still tagging for Layout Builder, even though it is a generic bug.
Comment #4
godotislateI don't whether this is the best way for a general solution, but this was my workaround:
TranslatableEntityLabelMarkup\Drupal\Component\Render\MarkupInterfaceand\Countable\Drupal\Component\Utility\ToStringTraitrender()method loads$entityfrom storage using entity type ID and entity ID, and returns$entity->label()admin_labelof relevant definitions to new instance ofTranslatableEntityLabelMarkup, passing in the entity type ID (e.g.,'block_content_type') and entity ID (e.g., block content bundle machine name)Edited to add: This was only for inline blocks for custom block content types. It probably doesn't work for field blocks at least.
Comment #6
tim.plunkettLet's start with a failing test.
Comment #8
berdirWe removed language specific caching of plugin definitions a pretty long time ago, we'll have to revisit that I guess. One reason we did that is that it allowed us to remove quite a few cache tags and tag-based invalidations. I think an alternative option that I posted there was to invalidate all known language suffixes explicitly instead.
Comment #9
godotislatePatch implementing solution described in #4 for inline blocks, field blocks, menu blocks, view blocks, and block content blocks.
This includes test in #6, but needs more tests to cover everything else besides menu blocks.
Comment #10
godotislateUpdated patch with fixes for failing tests.
Note that the placed blocks listed at /admin/structure/block are only shown in the default language, because they are loaded without config overrides. This is separative from the derivatives issue.
Comment #12
ilya.no commentedThanks for the patch. I have same problem and patch from #10 solves the issue.
Comment #13
s3b0un3tSame answer as ilya.no. Tested in 8.8.5.
Thanks a lot !
Comment #17
akalam commentedTested and worked as expected on D8.9. Thanks a lot @tim.plunkett and @godotislate for providing the patch.
Can somebody test it on D9 so we can move to RTBC?
Comment #18
berdirThis is awfully complex. Was there any discussion to instead cache block plugins per language? We used to do that, then removed it again but had a number of bugs due to that over time.
As a similar example, see \Drupal\Core\Entity\EntityFieldManager::getBaseFieldDefinitions().
Note that if we do that, we need a cache tag to invalidate plugin definitions in all languages on a cache clear, that's the downside. in the entity system, we have that already anyway.
Comment #20
unstatu commentedI confirm it works on 9.2.10. Setting to RTBC.
Comment #21
akalam commentedI'm setting this back to "Needs review" until the concerns expressed on #18 gets discussed and solved.
Comment #22
ranjith_kumar_k_u commentedRe-rolled # 10 for 9.4.
Comment #24
smustgrave commentedGetting this error after applying patch
Deprecated function: Return type of Drupal\Core\Entity\TranslatableEntityLabelMarkup::jsonSerialize() should either be compatible with JsonSerializable::jsonSerialize(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of core/lib/Drupal/Core/Entity/TranslatableEntityLabelMarkup.php).
Did need to clear cache after applying patch but I imagine that's to be expected.
Configured after clearing cache the block type links were showing as translated.
Also would be nice ot have a tests-only patch so we can confirm that worked
Did update the issue summary but didn't need it much.
Comment #25
ravi.shankar commentedFixed Drupal CS issues of patch #22, still needs work for #24.
Comment #26
smustgrave commentedComment #28
smustgrave commentedFixed the test case
Comment #30
penyaskitoI've seen same issue with menu links provided by modules. IMHO @Berdir suggestion at #18 is the way to go.
Comment #31
m.stentaDoes it make sense to update the title and component of this issue since it is not block-specific? @tim.plunkett mentioned that as well in comment #3:
I'm not sure what the "Component" should be. I'll set it to "language system" but please adjust if there's a better fit.
We encountered this with menu link derivatives that are created for each bundle of our entity type using
$bundle->label(): #3262752: Record type menu items lose translationsWrapping
$bundle->label()in$this->t()in our module fixes it - but that is not the "correct" solution, as I understand it.Comment #32
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #33
benjy44reroll for 9.5.x
Comment #35
akalam commentedI've found when you have a menu link derivative, and you are using an TranslatableEntityLabelMarkup as title, the title gets overridden by the static_menu_link_overrides, transformed into a string and not displaying the proper translated text.
I'm adding a patch with a fix so the title override skips in case it has been defined as a TranslatableEntityLabelMarkup.
Note: I've found the patch doens't apply anymore for the 11.x branch, so I'm uploading it as it is, just introducing the change mentioned above. The patch is applying on 10.1.x
Comment #36
akalam commentedRerolled against 10.2 and introduced a change to make it compatible with the change introduced on #3340159: Prevent empty block_content info fields from causing php deprecation notices when placing blocks with no label.
Comment #37
fromme commentedEntityType does not have
label()method, that why i get:Error: Call to undefined method Drupal\Core\Entity\ContentEntityType::label() in Drupal\Core\Entity\TranslatableEntityLabelMarkup->getTranslatedLabel() (line 233 of /var/www/html/web/core/lib/Drupal/Core/Entity/TranslatableEntityLabelMarkup.php).Updated patch: replaced
label()withgetLabel()Comment #38
tonibarbera commentedRerolled against 10.3.1. Tested only with 10.3.1
Comment #40
akalam commentedI've created a MR based on #38, but incorporating the changes introduced on #37 on getTranslatedLabel() method