Bug #78599

Copy localized content element leads to wrong reference, translation fails

Added by Markus Mächler about 3 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Must have
Category:
Localization
Start date:
2016-11-07
Due date:
% Done:

100%

TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
medium
Is Regression:
No
Sprint Focus:

Description

the following steps:

- create a new page "P.A"
- create a new content element "CE.A" on page "P.A"
- localize "CE.A" to "CE.A en"
- create a new page "P.B"
- add a new content element "CE.B" on page "P.B"
- copy "CE.A" to page "P.B" to "CE.A copy". this automatically copies "CE.A en" to "CE.A en copy".

the copied, translated element "CE.A en copy" does have a correct l18n_parent "CE.A copy" set, however, the t3_origuid points to "CE.A en", instead of "CE.A copy".

if you now hit the "translate" button on page "P.B", it results in "A en copy" being translated by the localization wizard over and over, leading to many copies of "A en copy" instead of translating "CE.B"

this was tested with TYPO3 7.6.12,7.6.13-dev without any extensions installed.


Related issues

Related to TYPO3 Core - Bug #75400: Related child elements don't get correct sys_language_uid during localization of parent record Closed 2016-04-04
Related to TYPO3 Core - Feature #78169: Add field for the record translation mode (connected vs free) Closed 2016-10-07
Related to TYPO3 Core - Bug #79443: Translation Wizard translates already translated content elements Closed 2017-01-24
Related to TYPO3 Core - Bug #83874: Localization manager wrongly chooses the same language to translate from as to translate to Closed 2018-02-13
Duplicated by TYPO3 Core - Bug #78701: The source language is wrong determined for copied record translations Closed 2016-11-15

Associated revisions

Revision a1abb8db (diff)
Added by Tymoteusz Motylewski over 2 years ago

[BUGFIX] Make LocalizationRepository handle copied records

Improve LocalizationRepository queries to handle case
when records were copied from another page (thus t3_origuid)
is pointing to records from the other page.

Now LocalizationRepository uses l10n_source field instead of t3_origuid.
Tests for LocalizationRepository covering the case were added.

Resolves: #79443
Resolves: #78599

Releases: master, 7.6
Change-Id: Ibae4a276ea814f0ce3d453cffef1d22afeff1eb9
Reviewed-on: https://review.typo3.org/51406
Tested-by: TYPO3com <>
Reviewed-by: Markus Mächler <>
Reviewed-by: Georg Ringer <>
Tested-by: Georg Ringer <>
Reviewed-by: Daniel Goerz <>
Tested-by: Daniel Goerz <>
Reviewed-by: Tymoteusz Motylewski <>
Tested-by: Tymoteusz Motylewski <>

Revision ddec3235 (diff)
Added by Markus Klein over 1 year ago

[BUGFIX] Make LocalizationRepository handle copied records

Improve LocalizationRepository queries to handle case
when records were copied from another page (thus t3_origuid)
is pointing to records from the other page.

Now LocalizationRepository uses l10n_source field instead of t3_origuid.
Tests for LocalizationRepository covering the case were added.

Resolves: #79443
Resolves: #78599
Releases: master, 7.6
Change-Id: Ibae4a276ea814f0ce3d453cffef1d22afeff1eb9
Reviewed-on: https://review.typo3.org/57628
Tested-by: TYPO3com <>
Reviewed-by: Johannes Kasberger <>
Tested-by: Johannes Kasberger <>
Reviewed-by: Stefan Neufeind <>
Reviewed-by: Andreas Fernandez <>
Reviewed-by: Łukasz Uznański <>
Tested-by: Łukasz Uznański <>
Reviewed-by: Tymoteusz Motylewski <>
Tested-by: Tymoteusz Motylewski <>

History

#1 Updated by Markus Mächler about 3 years ago

the proposed solution is to either

- set the origuid field of the copied localized record accordingly
or
- use transOrigPointerField in https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Domain/Repository/Localization/LocalizationRepository.php#L144

#2 Updated by Markus Mächler about 3 years ago

the records leading to the incorrect translation handling can be found with the following query,
which is used in https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Domain/Repository/Localization/LocalizationRepository.php#L34:

SELECT * FROM tt_content,tt_content AS tt_content_orig,sys_language
WHERE
tt_content.colPos = 0
AND tt_content.sys_language_uid = 1
AND tt_content.t3_origuid = tt_content_orig.uid
AND tt_content_orig.sys_language_uid=sys_language.uid
AND tt_content.deleted=0
AND (tt_content.t3ver_state <= 0 OR tt_content.t3ver_wsid = 0)

#4 Updated by Gerrit Code Review about 3 years ago

  • Status changed from New to Under Review

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50532

#5 Updated by Tymoteusz Motylewski about 3 years ago

Hi Markus,
Can you take a look if patch https://review.typo3.org/#/c/47645/ solves your issue?

#6 Updated by Markus Mächler about 3 years ago

hi Tymoteusz

i tried your patch and unfortunately, the problem still persists.

as far as i can understand, the problem lies in the usage of two different fields `l18n_parent` in sysext/core/Classes/DataHandling/DataHandler.php
and `t3_origuid` in sysext/backend/Classes/Domain/Repository/Localization/LocalizationRepository.php for the very same purpose.
IMHO this really needs to be fixed in order to avoid translation issues.

#7 Updated by Markus Mächler about 3 years ago

Imporant to notice: the Problem exists in TYPO3 7.6.12, 7.6.13-dev, as well as in TYPO3 8.5-dev, including the patch version https://review.typo3.org/#/c/47645/

#8 Updated by Tymoteusz Motylewski about 3 years ago

Thanks for the feedback.
I can reproduce the issue.

#9 Updated by Tymoteusz Motylewski about 3 years ago

Few notes:
- l18n_parent/l10n_parent/transOrigPointerField is always set to id of the record in the default language
- it is possible to translate content from different language than default. Because of that we can't just blindly replace t3_origuid usage with l18n_parent in the patch.

#10 Updated by Markus Mächler about 3 years ago

hi tymoteusz

thanks for your reply.

if you want to use t3_origuid, then we need to make sure it is correctly set when copying a content element. this is currently not the case as described in the initial issue message.

so how do you want to proceed?

#11 Updated by Oliver Hader about 3 years ago

We need to re-think the usage of t3_origuid as source for translations. This combined mode of localize & copy shows a severe conceptual flaw. Instead of adding another custom interpretation, this intentions need to be clear.

#12 Updated by Markus Mächler about 3 years ago

ok so who will fix this issue and how?

#13 Updated by Tymoteusz Motylewski almost 3 years ago

I spend some time investigating the current and past usage of the t3_origuid field. I wrote tons of functional tests for it to see how it behaves in different scenarios.
I found out that it's not so easy to use it consistently for localization purposes unfortunately (now it's used in translation wizard).
So right now we have 2 solutions
1. adapt the definition of t3_origuid and fix the core to align with this definition
2. introduce a new field `l10n_origuid` with a clear meaning.
Please take a look at the document describing the details of the issue and possible solutions.
https://docs.google.com/document/d/1rv_P8nO1F2VvBEk3Tbb5zH6LuCXHUsgMo0csde6NGUQ/edit#

So far most of the core team is leaning for having a new field and use it in the translation wizard. - I've opened an issue for it
https://forge.typo3.org/issues/78169

However to improve the current situation in v7 we could do change the current queries done in translation wizard to be "more inteligent" and detect whether t3_origuid has usable value or not.

#14 Updated by Markus Mächler almost 3 years ago

hey Tymoteusz

thanks for this excellent analysis!
as far as I am concerned about 7, I wrote an xclass implementing the changes i submitted in the patch, replacing the usage of t3_origuid with l18n_parent in the translation wizard, which works fine so far.
i suggest to have this fix for 7, as it is an easy and quick solution. in the case of records being translated from a record in a non-default language, it should not require too much additional code which also handles this case.

#15 Updated by Tymoteusz Motylewski almost 3 years ago

The rough idea how to solve it would be:
1. First check if a record has l10n_parent (transOrigPointerField) set
2. if yes, use it
3. if not try to use t3_origuid, but double check if t3_origuid has correct value before using it
- it should be different than record uid
- target record (the one t3_origuid is pointing to) should be in different language then the current record

#16 Updated by Markus Mächler almost 3 years ago

thanks for your suggestion, it sounds reasonable.

will you implement this?

#17 Updated by Tymoteusz Motylewski almost 3 years ago

I'm working on fixing other issues right now, so I will not be able to implement this in the near future.
If you have time, feel free to improve your patch.

#18 Updated by Markus Mächler almost 3 years ago

hey Tymoteusz

ok, i will try to improve the patch in order to fit your requirements.

#19 Updated by Tymoteusz Motylewski almost 3 years ago

awesome :)

#20 Updated by Gerrit Code Review over 2 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51406

#21 Updated by Gerrit Code Review over 2 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51406

#22 Updated by Tymoteusz Motylewski over 2 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#23 Updated by Markus Mächler over 2 years ago

This still has not been backported to 7.6

#24 Updated by Gerrit Code Review over 1 year ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#25 Updated by Gerrit Code Review over 1 year ago

Patch set 2 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#26 Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#27 Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#28 Updated by Gerrit Code Review over 1 year ago

Patch set 5 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#29 Updated by Gerrit Code Review over 1 year ago

Patch set 6 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#30 Updated by Gerrit Code Review over 1 year ago

Patch set 7 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#31 Updated by Gerrit Code Review over 1 year ago

Patch set 8 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#32 Updated by Gerrit Code Review over 1 year ago

Patch set 9 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#33 Updated by Gerrit Code Review over 1 year ago

Patch set 10 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#34 Updated by Gerrit Code Review over 1 year ago

Patch set 11 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#35 Updated by Gerrit Code Review over 1 year ago

Patch set 12 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#36 Updated by Gerrit Code Review over 1 year ago

Patch set 13 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#37 Updated by Gerrit Code Review over 1 year ago

Patch set 14 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/57628

#38 Updated by Markus Klein over 1 year ago

  • Status changed from Under Review to Resolved

#39 Updated by Tymoteusz Motylewski over 1 year ago

  • Related to Bug #83874: Localization manager wrongly chooses the same language to translate from as to translate to added

#40 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF