Bug #88784

Record language is incorrectly changed to default language in overlayLanguageAndWorkspace

Added by Jonas Schwabe 5 months ago. Updated 5 months ago.

Status:
Under Review
Priority:
Should have
Assignee:
-
Category:
Extbase
Target version:
-
Start date:
2019-07-16
Due date:
% Done:

0%

TYPO3 Version:
9
PHP Version:
7.2
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

When a record with language uid !=0 is processed while the current language uid is 0 overlayLanguageAndWorkspace incorrectly "transforms" the record into the default language.

The relevant code is as far as I can tell this one:

if (isset($GLOBALS['TCA'][$tableName]['ctrl']['transOrigPointerField']) && $row[$GLOBALS['TCA'][$tableName]['ctrl']['transOrigPointerField']] > 0) {
  //force overlay by faking default language record, as getRecordOverlay can only handle default language records
  $row['uid'] = $row[$GLOBALS['TCA'][$tableName]['ctrl']['transOrigPointerField']];
  $row[$GLOBALS['TCA'][$tableName]['ctrl']['languageField']] = 0;
}

A foreign language record is handled by overlayLanguageAndWorkspace so the condition passes. The language uid is set to 0 for this record, the record is then considered as main language. When it is next passed into getRecordOverlay it's not being dropped (as it should be in this case).

The above "fake" should only happen if $languageUid is not 0:

if (isset($GLOBALS['TCA'][$tableName]['ctrl']['transOrigPointerField']) && $row[$GLOBALS['TCA'][$tableName]['ctrl']['transOrigPointerField']] > 0 && $languageUid > 0) {
  //force overlay by faking default language record, as getRecordOverlay can only handle default language records
  $row['uid'] = $row[$GLOBALS['TCA'][$tableName]['ctrl']['transOrigPointerField']];
  $row[$GLOBALS['TCA'][$tableName]['ctrl']['languageField']] = 0;
}

This issue becomes visible e.g. when relations are resolved as in tx_news: https://github.com/georgringer/news/issues/843#issuecomment-511815126

wdb_language_hook.zip (11.3 KB) David Bruchmann, 2019-07-16 17:39

History

#1 Updated by David Bruchmann 5 months ago

  • File wdb_language_hook.zip added

Jonas,
I'm interested if your issue can be solved with the attached extension. would be nice if you could try it and give feedback.
Installation is done either by composer or by extensionmanager, afterwards nothing is to do, just clear cache and test it.

#2 Updated by Jonas Schwabe 5 months ago

It gives me a syntax error (the semicolon in typo3conf/ext/wdb_language_hook/Classes/Hooks/Frontend/Page/PageRepository.php line 131).

#3 Updated by David Bruchmann 5 months ago

Ohh I'm sorry, you can just remove the semikolon in that line.

#4 Updated by David Bruchmann 5 months ago

  • File deleted (wdb_language_hook.zip)

#5 Updated by David Bruchmann 5 months ago

I uploaded the extension without that semikolon and deleted the wrong version.

#6 Updated by Gerrit Code Review 5 months ago

  • Status changed from New to Under Review

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

#7 Updated by David Bruchmann 5 months ago

In the attached extension you can find how fake-entries are used very targeted for powermail.
powermail is using it or something similar by itself to produce in frontend forms with the same html-attribute 'name', so there is a reason for it.
concerning powermail the extension is not working like expected but that's another issue.

My point is that fake-entries should be used in very special cases - if at all - and extbase is doing it completely wrong in my opinion.
I never down-voted the patch but think that the whole construction with fake-entries is a real burden for the whole TYPO3 system.

#8 Updated by Jonas Schwabe 5 months ago

I will have a look at the extension later (just needed to get this done).

What exactly do you mean by fake entries? No entry is being created here. Can you specify what powermail does?

#9 Updated by David Bruchmann 5 months ago

With fake-entries I mean that a value to a row is assigned that isn't like that in the table:

$row[ $GLOBALS['TCA'][$tableName]['ctrl']['languageField'] ] = 0;

This construction makes proper handling by the core much more more difficult as it's intended to circumvent the core-handling, which is in my opinion the wrong way.

#10 Updated by David Bruchmann 5 months ago

The powermail problem is this:

You create a form, including a powermail-page and fields.
In the original language for fields you create markers which are saved in the database.
Translated records never have these marker-fields but take the markers from the original language. I never found out yet how powermail is doing it in detail, but the purpose is to have in Frontend the same markers, no matter about the language. If markers are different (that can happen if the records are wrong handled by a hook, or even in some fall-back situations perhaps) the forms never work. If powermail-forms never work it can be remarked when the form is shown again after sending instead of showing a response-page.

In my extension I called these fields `intrinsicFields` and they could be configured for every extension - but very targeted and not in general.

Concerning powermail these `intrinsicFields` are handled too late by the hook I used, but the extension serves as example how a targeted usage could be realized in general.

#11 Updated by Gerrit Code Review 5 months ago

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

#12 Updated by Jonas Schwabe 5 months ago

$row[$GLOBALS['TCA'][$tableName]['ctrl']['languageField']] = 0;

Does not create a fake record. It's actually used to prime the core language overlay because this only works when the main records is passed.
The other option would be to drop the above code block completely and ignore every relation that does not refer to the base record.
I think the idea with the current code might be, that sometimes relations do not refer to the base record (e.g. because there was no foreign_table_where in a select TCA config that makes sure only sys_language_uid = 0 or l10n_parent = null records are selectable).
Dropping this handling would therefor lead to non resolvable relations in this case.

Regarding powermail: Using getRecordOverlay_postProcessshould work because if there is a translation it should be fetched from database and contain the correct sys_language_uid once passed to getRecordOverlay_postProcessshould as far as I understand the code.

Your extension hooks into getRecordOverlay_preProcess where sys_language_uid will always be 0 if the row comes from extbase. But this is the only time getRecordOverlay does something other then just returning the same data or dropping the row completely if there is a mismatch.

#13 Updated by Tymoteusz Motylewski 5 months ago

Hi All,
Thanks a lot for putting effort in this area!
The language handling topic is complex and its hard to fix stuff without breaking something else. I've put some months of work to improve the situation for v9 and writing tons of tests. I'm very happy to see other people also caring for it.

Before continuing the discussion, please take a moment to read the concept of the new language handling in v9.5, so we're sure we are on the same page:

https://docs.typo3.org/c/typo3/cms-core/master/en-us/Changelog/9.5/Important-82363-MakeExtBaseTranslationHandlingConsistentWithTyposcript.html

Also a good start is to see how stuff works and what is missing in the tests, please check out functional tests:
extbase/Tests/Functional/Persistence/QueryLocalizedDataTest.php
extbase/Tests/Functional/Persistence/TranslationTest.php

I'm open for scheduling a call to discuss the topic with whoever is interested.

Please also note that depending on the relation type and configuration uid's used in the relation are sometimes default language record uids and sometimes translated record uids (e.g. in inline relations).
See a summary here for details:
https://docs.google.com/document/d/1lfJqotm_omkqIJTgo4xaHteDreBUQ9_Z1_9T6nomv9c/edit#

#14 Updated by Gerrit Code Review 5 months ago

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

#15 Updated by Jonas Schwabe 5 months ago

I thought about this a little further.
The problem here is, that there is a relation between a non translated and a translated record. In this case there are actually two relations (one to the original language and one to the translated version). This happened for some reason when we translated sys_categories (relations were also "translated").
So in addition it would probably be a good idea make sure that only relations to the original record or records that have no translation and are of the current languge can be found through a relation - similar to what happens in the repository when initially loading records.

But sure we can schedule a call if that would help to sort this out :) (maybe via Slack?)

Also available in: Atom PDF