Problem/Motivation

If you translate the literal "inline blocks" to another language in the layout builder, it stops working.

This is because in the controller, there is a validation with the literal "Inline blocks".

/web/core/modules/layout_builder/src/Controller/ChooseBlockController.php line 157 "if (isset($blocks['Inline blocks'])) {"

Proposed resolution

Translate the string before using it.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

julenmelgar created an issue. See original summary.

julenmelgar’s picture

Issue summary: View changes
julenmelgar’s picture

Issue summary: View changes
tim.plunkett’s picture

Version: 8.6.4 » 8.7.x-dev
Category: Feature request » Bug report
Issue summary: View changes
Status: Active » Needs review
Issue tags: -Layout Builder, -translate, -content translation +Blocks-Layouts, +Needs tests
FileSize
1.1 KB

This is because of how \Drupal\Component\Plugin\CategorizingPluginManagerInterface::getGroupedDefinitions() works: it uses translated strings as array keys. Not great :(

This needs a test, but here's the fix.

julenmelgar’s picture

Hello @tim.plunkett, first thanks for your quick reply and your contributions. I'm doing some tests to see if the patch works completely well.

Kristen Pol’s picture

I agree with @julenmelgar that the patch in #4 looks good. I'll try to test now.

amit.drupal’s picture

@tim.plunkett, Test patch #4 it's working fine.

Kristen Pol’s picture

I've been unable to test this with simplytest.me. Tried on various occasions. @amit.drupal how did you test on your local?

Kristen Pol’s picture

Status: Needs review » Needs work
FileSize
294.25 KB

Hmm... I am not seeing "Inline blocks" in the list of strings. I must be looking in the wrong place. Please add steps to reproduce this to the issue summary. Thanks.

Ismail Cherri’s picture

Status: Needs work » Reviewed & tested by the community

@Kristen Pol
Steps to reproduce:
1. Install Drupal
2. Enable content, config and interface translation
3. Add additional language e.g. German & Import translations
4. Add a new custom block type (/admin/structure/block/block-content/types/add) e.g. Test Block
5. Enable Layout Builder module
6. Enable it for Basic Page content type
7. Create a new basic page in English
8. Add an inline block -> works as you see a list of inline blocks
9. Switch to German interface
10. Add an inline block -> Does not work as you don't see the list of inline blocks

Notes
- The issue doesn't happen if there only one type of inline blocks especially the default one
- I couldn't apply the patch in #4 with simplytest.me also.
- I applied it on a fresh Drupal 8.6.9 and 8.7.x-dev installations and it fixes the issue

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Looks like we need an automated test here. Thanks!

tim.plunkett’s picture

Locale module + content translation + 2 content blocks all to test an artifact of the plugin system. Idk if I've ever written a more lopsided test :)

FAIL patch is also the interdiff

The last submitted patch, 12: 3019333-translation-12-FAIL.patch, failed testing. View results

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Perfect.

xjm’s picture

+++ b/core/modules/layout_builder/src/Controller/ChooseBlockController.php
@@ -168,8 +168,9 @@ public function inlineBlockList(SectionStorageInterface $section_storage, $delta
+    $inline_blocks_category = (string) $this->t('Inline blocks');
+    if (isset($blocks[$inline_blocks_category])) {

So, definitely not a fan of this -- stringcasting a translatable string that varies per language for an array key instead of keying on a machine name is brittle and bug-prone -- but per @tim.plunkett this is unavoidable because of the way the API works. See #4. I filed #3032836: CategorizingPluginManagerTrait::getGroupedDefinitions() keys groups by a stringcast of a translatable string, which is brittle.

I also asked if we needed a cache clear. @tim.plunkett pointed out that this line's method call would be cached:

+++ b/core/modules/layout_builder/src/Controller/ChooseBlockController.php
@@ -168,8 +168,9 @@ public function inlineBlockList(SectionStorageInterface $section_storage, $delta
     $blocks = $this->blockManager->getGroupedDefinitions($definitions);

...but fortunately, we're not changing that line, just the build array. I also asked whether the render cache would need to be invalidated for that, but (again, per @tim.plunkett) the Layout Builder UI is already marked as uncacheable.

Saving issue credit.

  • xjm committed 60bd499 on 8.7.x
    Issue #3019333 by tim.plunkett, julenmelgar, Ismail Cherri: If you...
xjm’s picture

Version: 8.7.x-dev » 8.6.x-dev
Priority: Normal » Major

Committed and pushed to 8.7.x. Thanks for reporting this! Promoting to major since this basically breaks the inline blocks feature for non-English sites.

This is probably eligible for backport since the impact outweighs the disruption, but I'm going to queue an 8.6.x test run before pushing that cherry-pick.

  • xjm committed ea5dec7 on 8.6.x
    Issue #3019333 by tim.plunkett, julenmelgar, Ismail Cherri: If you...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

And pushed the 8.6.x cherry-pick! Thanks all.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.