Needs review
Project:
Translation Management Tool
Version:
8.x-1.x-dev
Component:
Core
Priority:
Major
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
29 Nov 2019 at 16:05 UTC
Updated:
28 Aug 2026 at 17:01 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
miro_dietikerI agee that this integration is technically possible for an initial translation.
However when i investigated those layout builder content blocks they looked like non-reusable, non-revisionable and technically untranslatable. As a result, any such block added in a translation is disconnected from the original. It is not possible to remap them with additional update translation cycles or detect changes. (Back then they were also lost when you added a translation manually and needed to be manually added...)
Thus the tmgmt promise of "managing translation" is not possible... in fact, in my opinion, the whole feature is technically broken as it doesn't properly integrate with Drupal Core translation workflows at all.
Did something change about this meanwhile? If not, you might want to raise awareness and priority in addressing these problems.
The first initial translation is only a tiny fraction of the problem..
Comment #3
nod_I see, it seems we can see even inline blocks in the source list in the custom blocks code list
/admin/tmgmt/sources/content/block_contenteven if on the admin list they don't show up. It feels like it's less dramatic than it seems no?Comment #4
berdirYes, they are shown there an can be translated manually, but what Miro means is that layout_builder itself does not properly support translations. Until #2946333: Allow synced Layout override Translations: translating labels and inline blocks is resolved, support in TMGMT is going to be very limited. Definitely if you also add content_moderation.
Once core fully supports translations, we could automatically include blocks like we do paragraphs.
Comment #5
nod_Gotcha, thanks for the details!
Comment #6
miro_dietikerI hear that
There is a solution that layout builder can contain symmetric translated non-reusable blocks
https://www.drupal.org/project/layout_builder_st
Usage already at 3000+.
One problem is that the block + revision IDs are serialilzed on the layout aware host entity field. So they need to be updated from the host perspective. The block reference to save the block is not enough.
I think that we could "easily" create a plugin that provides these block fields as data items and save them back. And it seems with that symmetric approac, things also won't fall apart?
Comment #7
miro_dietikerWhile the plugin proposal with support for l ayout_builder_st seems pretty straight forward, the impact of combining it with content moderation will need to be tested. It's common that many unexpected things come up with it...
Comment #9
berdir@Shawn Conn: I see you started a merge request to implement this.
I'm not sure if I'd implement this in the way you did. My rough idea was to handle it like paragraphs, so we'd automatically inline the translations into the same job item if they are non-reusable. For reusable blocks, we could support it through the suggestions API and allow them to be added additionally.
Comment #10
shawn conn commented@Berdir first time using the new Gitlab merge fork process as I needed someplace to put a WIP patch for testing. When I created the fork, I wasn't expecting it to open notifications on the page. I guess the behavior is a little different from GH.
Anyway, there are some subsystem parts of TMGMT I'm not familiar so I'd defer to you on what's the more comprehensive solution here. This WIP patch was to expedite a temporary solution until #2946333 unblocks this issue.
Comment #14
kmontyI added Content Moderation support for the WIP Layout Builder patch. While I know based on comment #9 that this patch isn't the approach the maintainer would ultimately like this issue to take, I figured this change could help folks already using the patch.
Comment #15
aspilicious commentedThe current patch in this issue doesn't update the "master entity". The referenced issue does it after I updated the patch but only for nodes.
THIS patch removes the node specific checking and makes it work for every content entity.
I tested this with a block type that had layout builder support.
Comment #16
aspilicious commentedThat was the node patch, here is the content entity interface version.
Comment #17
gantal commentedI am currently experiencing this issue and tried the patch in #16 but it seems like that patch adds some missing references in
TmgmtJobItemSubscriber.php. For example, theTranslatableSectionStorageInterfaceclass does not exist andContentEntityInterfaceobjects don't have thegetKeysmethod. Is there perhaps another patch that needs to be applied before this one?Comment #18
duwid commentedFor everyone using layout_builder_st module instead of the patch from 2946333, here is an updated version of this patch, that will fix the following error message:
Error: Undefined class constant 'TRANSLATED_CONFIGURATION_FIELD_NAME' in Drupal\tmgmt\EventSubscriber\TmgmtJobItemSubscriber->updateSectionComponents() (line 141 of modules/contrib/tmgmt/src/EventSubscriber/TmgmtJobItemSubscriber.php).Comment #19
aspilicious commented@gantal the getKeys stuff was indeed a mistake.
Here is an updated patch for that line.
Comment #21
phma commentedAs we have to keep multiple implementations in mind for now, I suggest we use different event subscribers depending on which layout builder translation module is enabled (or in case core has been patched).
I'm currently looking into supporting layout_builder_at (wished we would have gone with layout_builder_st or the core patch instead...). The code shouldn't break anything, but it also doesn't update the node yet. It's possible that the block of the source node is being overridden, though, so use with care! I wasn't able to test code with layout_builder_st, so if @aspilicious or @Duwid could check it, would be great!
To @aspilicious and others: Is it possible to stick to the issue fork and not post patches moving forward? It makes it hard to keep track of the changes and merge them back into the fork. You can easily request push access and contribute your changes to the fork.
Comment #22
phma commentedlayout_builder_at only works, after this patch is applied:Applying patch #3286719: Fails on translated inline blocks for layout_builder_at is highly recommended if you want to also be able to add translations directly without using a tmgmt job. Also, to be able to keep editing the source blocks, the latest patch from #3053881: Reverting entity revisions that contain custom blocks erroneously triggers EntityChangedConstraint is needed.I did notice that strange things start happening when reverting revisions, so watch out.EDIT: #3053881 might no longer be needed, as I'm also updating the revision number in the latest commit (needs testing). Also, I didn't encounter any issues with loading and reverting older revisions any more.
Comment #23
odai jbr commentedThe patch works fine with TMGMT but it fails when you have a nested inline blocks as it doesn't create new copies from the children blocks.
Comment #24
johnrosswvsu commentedI can confirm that the nested inline blocks are being sent and received but once the entities are saved the nested inline block is not saved. Or if they are saved the wrong revision or target_id is used in the translation.
Comment #25
larvymortera commentedHere's an updated patch addressing the nested inline blocks issue.
Comment #26
johnrosswvsu commentedCan confirm that the patch on #25 applies cleanly and that the nested (child) inline blocks are translated.
Comment #27
johnrosswvsu commentedPlease disregard this patch.
Comment #28
johnrosswvsu commentedComment #29
odai jbr commentedthere is another issue in the patch, a huge number is created from each translated block... after translating a page that has a layout containing 4 blocks only we found that the blocks has been created multiple times, same content and everything but duplicated many times, the layout used the latest translated blocks but all the other duplicates are created for nothing.. we found this after we noticed the site became too slow because of the huge number of blocks..
Comment #30
ahmad khader commentedThis patch addresses the following issues within the module and the above patches, specifically tested on 'layout_builder_at':
1. Remove Duplicated Inline Blocks.
2. Separate Inline Block Translations with Unique IDs:
Each inline block's translation is assigned a unique ID, intricately connected with the original ID within the configuration. This prevents inline blocks from disappearing in the source language when updating other translations. Notably, the configuration of the component's fixability becomes crucial, as it ensures sensible handling of inline-block translations.
3. Restrict Display of Inline Blocks on Source Page:
Given that inline blocks are inherently connected to entities and cannot be individually translated, there is no need to display them on the source page. This restriction streamlines the visual presentation of the source page.
4. Enhance Functionality of Inline-Block Translations and Nested Inline Blocks:
Bugs have been addressed, and new features have been added to optimize the translation process:
- Blocks not present in the source language can now be retained without disappearing during translation requests.
- Adding a new block in the source language and subsequently requesting translation via TMGMT will only include the newly added block and they will appear on the bottom of the page to void conflicts with other blocks.
- For retranslation needs, simply deleting a block from the translated entity and requesting translation will seamlessly add it again.
It's important to note that this patch has undergone testing exclusively on 'layout_builder_at'. If you have further questions or need additional details on the implementation, feel free to inquire.
Comment #31
ahmad khader commentedsupport-lb-with-inline-block-3097660-30.patch with a ticket issue in it is the right patch.
Comment #32
ahmad khader commentedFixes to #31 patch
Comment #33
ahmad khader commentedcontinuation of patches #30-#32 fixes.
This patch addresses the error Drupal\Core\Entity\EntityStorageException: Invalid translation language (es) specified. in
, caused by non-Block content type entities accessing the code.
Comment #34
nginex commentedGuys,
Please do not refactor/fix coding standards of non-related code to this issue, you are causing at least a merge conflict and a lot of mess, this is not a scope of the issue, it's a purpose of a separate issue in the project
Comment #36
heddnI've reverted the out of scope changes in the MR. Unfortunately, none of the patch changes since about comment #23 are in the MR. If there are features in those patches that need to make there way into the MR, please post to it instead of a new patch.
Comment #37
heddnI think (hope) the JS test failure is a random failure. But everything else is green again.
Comment #38
heddnDug into the last failing test (JavaScript). After running it several times on local, I've come to the conclusion it is a random failure. After all that, it also now runs green on the testbot. The MR is now ready for a round of reviews.
Comment #39
heddnOne note about the code in the MR. "Add to cart" does not select layout blocks. Only "Request translation" selects the layout blocks.
Comment #41
marvil07 commentedIndeed, that was the case.
I have added support for that.
Sadly I needed to do that with a work-around.
For some reason the form state value related to layout builder toggle is not available on the cart submit callback.
So, I have added a form state arbitrary value based on the actual value of the toggle, at the validation stage, which is the last I moment I see it there.
Comment #42
nicxvan commentedI just wanted to comment that this only supports content blocks, it would be nice if it supported config blocks too.
I'm working on it locally and will submit it back, but I'm leaning heavily on how lingotek did it here: #3180664: Layout builder symmetrical translations support
Even cutting out the nice symmetrical vs asymmetrical handling locally it seems to pull in the config blocks.
Setting this to needs work, but if that is out of scope for this issue let me know.
Comment #43
nicxvan commentedHaving worked with this for a bit, this isn't the right issue for what I mentioned in 42.
I'm doing a review now.
Comment #44
finex commentedHi, I've tried the MR2. It looks promising but I've to report a couple of issues:
Translation of key values should not be allowed
Key field values should not be translated. For example, I have a block with a custom field called "image position" that has three possible values: top, left, and right. The field itself is translatable because, using AT, the image position may vary between languages.
However, MR2 translates the field’s values when it should not, as these values must remain unchanged.
Extreme duplication of records
Another major issue I've encountered is the extreme duplication of records. I tested this on a trilingual website by creating a basic page with a custom layout. The layout contained twelve sections and 26 blocks. Before the translation process, the block_content table had exactly 26 records, one for each block. However, after translating the page into the other two languages, the table grew to more than 40,000 records.
Comment #45
ahmad khader commented@finex,
I fixed the duplication of records on #30-#33 patches with other improvements.
perhaps we should include the patch in the MR!
Comment #46
nicxvan commented@ahmad khader yes please do!
Comment #47
finex commentedThank you @ahmad khader.
I will try the new MR.
Comment #49
camvertiz commentedWhen the
tmgmt_get_layout_builder_inline_blockshook was replaced bytmgmt_retrieve_layout_builder_inline_blocks, the changes introduced in #13 were inadvertently lost. I've re-applied those changes to restore content moderation support in the feature.Comment #51
camvertiz commentedIntegrated the changes from patch #33 into the merge request (see comment #45). Also the pipeline was failing because the specified Webform versions (
6.1.x-dev||6.2.x-dev) require older versions of Drupal core (up to10.x), which are incompatible withdrupal/core-recommended 11.1.7. I updated the Webform version to fix this.Comment #53
kmontyI'm spinning off the composer issue into its own item here #3524981: Webform version constraints in TMGMT's Composer file are breaking builds
Comment #55
jhedstromI addressed some PR feedback and am adding a patch file here for use with composer. So far in testing this is working fantastically!
Comment #56
papagrandeThe patch in #55 applies cleanly to both v1.16.0 and v1.17.0. Thanks, @jhedstrom.
Comment #57
hmdnawaz commentedI have tested the patch from MR.
I have created a custom block in my module
\Plugin\Block\MyBlock, that extends the BlockBase.I have added that block to the layout builder, but it is not included in the translated version.
Similarly, for block_content. I have added an inline block, and the same applies to this block as well.
In the function
tmgmt_retrieve_layout_builder_inline_blocksthere is a checkif (isset($component_config['block_revision_id'])), the block config doesn't haveblock_revision_idboth for custom and block content and because of that this function returns an empty array.And one more question: Is this issue only applicable to block_content, and is there no support for custom blocks?
Comment #58
phjouThe patch from #55 works very well for the most part.
I do have an issue for block titles, when accepting changes, the "Block description" field in the interface doesn't seem to be used and doesn't update the block title.
Comment #59
mpereztejeiro commentedThat was the issue for us as well but we made another patch that introduced this in
/src/EventSubscriber/AsymmetricTmgmtJobItemSubscriber.phpwith a patch:I post here the patch we came up with to fix this.
Comment #60
kmontyThis requires a manual rebase due to merge conflicts.
Comment #61
phjouI encountered a bug.
The TmgmtJobItemSubscriberBase::onAccepted() method incorrectly passes the job item's entity ID to InlineBlockUsage::getUsage() regardless of the job item's entity type. This causes unintended behavior when a node ID happens to match an existing block_content ID.
Steps to Reproduce
So I just added a test to make sure that we are managing only block_content.
Please note I created my patch from #55 since the merge request seems outdated. So the diff file is with #55.
Comment #62
phjouOk I have discovered an issue that was crashing my site when translating pages with blocks that had paragraphs fields.
When using createDuplicate, the paragraph is not saved yet, so only the entity is available. The target_id and target_revision_id don't exist yet and end up with a crash when trying to load the paragraphs using those.
The issue exist in layout_builder_at and there is a patch for it:
#3541778: It's possible a referenced paragraph does not exist, resulting in a WSOD
I've just added support for that issue and also integrated the change in #59
Comment #63
kmonty@phjou Can you update the merge request with your changes?
Comment #64
trafo commentedSome changes from patch #33 were not added to MR. Notably
source_block_idis not set in layout section component and used in `AsymmetricTmgmtJobItemSubscriber`. But it is still used intmgmt_get_translated_inline_blocks.Comment #65
vasikeI'm curious if there's anyone make it work with the latest
layout_builder_atrelease?including the fix mentioned in the latest #62 patch?
for me ... in "custom drupal instance" it doesn't do a thing about the layout builder data ... empty.
Comment #67
vasikeOk ... I figure out why the "Asymmetric Translation" won't work out of the box:
- Asymmetric Translation does not assume the content entities used are translatable - have Content Translation enabled
- TMGMT - assumes in its code that usually the content entities are translatable ... including the paragraphs (embedded/referenced entites) or in this case inline blocks ...
So I could use this module and the MR, to translate content with Layout Builder Asymmetric Translation.
But I had to do some updates:
- Enable translation for possible entities that gets in a complex Layout Builder "structure"
and on the MR:
- rebased
- Fix block id identifier for tmgmt_get_translated_inline_blocks helper function
- Fix PHP error
ContentTranslateFormsubmit Exceptions.- Update
AssymeticTmgmtJobItemSubscriberwith latest code from layout_builder_at module ... that actually works.- Extra: some PHPCS fixes.
Probably still needs work ... but for now, let's Review
Comment #68
vasikeSmall update on layout_builder_at integration
I created new task there ... to provide "API" that can be used in other places ... including here
https://www.drupal.org/project/layout_builder_at/issues/3577890
And I tried using in this issue MR in
AsymmetricTmgmtJobItemSubscriberand it seems it works ...Maybe it could be helpful not to maintain other module specific implementations ...
Comment #69
kmonty@vasike I reviewed the latest commits and they look good.
Scanning over the PR as a whole, it looks like we're still introducing a number of new SAST majors (particularly
\Drupal calls should be avoided in classes, use dependency injection instead), but that seems to be an existing pattern in the module as a whole. Not positive if that is a requirement for a merge or not.I went back and resolved my two open change requests, as they have been attended to.
Comment #70
vasike@kmonty thanks for review/updates on MR
about coding ... this module is still on early "D8 shape", I would say ... so I stopped me continue with such updates as ... the MR would be huge, imho.
As I said in my previous comments ... I only checked Asymmetric Translation, so It would nice to have also checked the Symmetrical one - https://www.drupal.org/project/layout_builder_st
Question ... Wondering if for this issue a similar approach as for EntityReference fields (
getEmbeddableFields) could be an option ... then separate job items ...Comment #71
vasikeI discovered a problem with Asymmetric translation
AsymmetricTmgmtJobItemSubscriberis running for all inline blocks ... so it runs multiple which means will create/recreate the sections and new inline blocks that leads to some data duplications and maybe also loosing translations done ... from previous iterations as there is some cleanup in the process.So we need for the Asymmetric Translations to operate at the Job level and not Job item level ... and only on the parent entity and not inline block (content block)
And I pushed a new commit for this refactor.
And another question/issue about this topic ... what happens with entities and their data for
entity_reference_revisionsin the process.Could it "dirty the DB" ?!?!
some testing and feedback would help. thanks
Comment #75
kmonty@yazanmajadba If you have proposed changes to the existing MR, can we just focus the work in MR!2? There does not seem to be a need to propose two different solutions for maintainers yet.
Comment #76
yazanmajadba commentedI tried several patches here, but they caused duplication issues. I refactored patch #33 to work with D11 and added a few enhancements. I tested it, and it worked successfully for me.
Comment #77
hkumar4 commented@yazanmajadba your changes doesn't include the latest changes done by @vasike. Can you please add your updates to the existing MR!2?
Comment #78
hkumar4 commentedHi @vasike can you please let me know if existing MR works well for async translations or it will need further tweaking.
Comment #79
ao5357 commentedI believe this is a correctly re-rolled patch of #76 to cleanly apply to 8.x-1.19