Bug #77298

Wrong language overlay logic in extbase

Added by Markus Klein almost 3 years ago. Updated 8 months ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Extbase + l10n
Target version:
-
Start date:
2016-07-29
Due date:
% Done:

0%

TYPO3 Version:
6.0
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

setup

Model A has a relation to model B (eg categories).
Model A can be translated.
The relation to B is only maintained in the default language
We assume content_fallback mode.

action

Fetching translated records for A which are assigned to B with uid 1.

expected result

A list of A-records.

actual result

Empty result

issue

Invalid SQL code generated in Typo3DbQueryParser::getSysLanguageStatement

affected versions

6.0 to 8

example

ext:news with categories.

  • Create a record in default language
  • Assign a category to your news.
  • Translate the news (but do not a assign a category in the translation, since this should be fetched from default lang)
  • Create a plugin and select a category filter there

The result is "no news found".

This is an extraction of the invalid SQL generated with comments inside.

# limit the result to the selected category uid 1 only
# this part only allows for news records of default language (correct query part!)
tx_news_domain_model_news.uid IN (
    SELECT uid_foreign FROM sys_category_record_mm 
    WHERE uid_local='1' AND sys_category_record_mm.fieldname = 'categories'
    AND sys_category_record_mm.tablenames = 'tx_news_domain_model_news'
)

# this part is the killer
AND (
   # either we allow only translated records (which never have a direct category relation)
    tx_news_domain_model_news.sys_language_uid IN (1,-1)
    OR (
  # or we allow news records, which do not have a translation in the language uid 1 at all (bad luck if all news are properly translated)
        tx_news_domain_model_news.sys_language_uid=0
        AND tx_news_domain_model_news.uid NOT IN (
            SELECT tx_news_domain_model_news.l10n_parent
            FROM tx_news_domain_model_news 
            WHERE tx_news_domain_model_news.l10n_parent>0
            AND tx_news_domain_model_news.sys_language_uid=1
            AND tx_news_domain_model_news.deleted=0
        )
    )
)

remarks

It is completely wrong to select records directly in the translated language in an overlay scenario as any other where condition (eg category selection above) of the query, which covers relation filtering, will always relate to the default language record. There current SQL excludes exactly those default records (since they have a translation) and all those where clauses fail.

The code was introduced with https://review.typo3.org/10188

external resources

http://www.dmitry-dulepov.com/2015/01/strange-code-in-extbase-persistance.html


Related issues

Related to TYPO3 Core - Bug #32216: OrderBy on translated records doesn't work properly Closed 2011-11-30
Related to TYPO3 Core - Bug #82363: Make Extbase translation handling consistent with typoscript Closed 2017-09-07
Related to TYPO3 Core - Bug #63694: extbase: strict mode and bug with queries in regards addSysLanguageStatement - e.g. contains ignores different MM translations Closed 2014-12-09

History

#1 Updated by Markus Klein almost 3 years ago

  • Description updated (diff)

#2 Updated by Markus Klein almost 3 years ago

When thinking about a solution keep in mind that relations to records may be defined on the default language and optionally as well on a translated record. So the selection logic must take care about all those possibilities.

#3 Updated by Markus Klein almost 3 years ago

A note on "order by on translated records". This is simply NOT possible to achieve on DB level when overlays are in the game, due to l10n_mode etc.

#4 Updated by Markus Klein almost 3 years ago

  • Description updated (diff)

#5 Updated by Dmitry Dulepov almost 3 years ago

Thank you for the excellent description of the problem.

There are two quirks with this problem.

First of all, when Kasper designed localisation model for TYPO3, it was always default language + translation. It was described in the Frontend Localisation Guide and every localisation-related code in TYPO3 followed it strictly. It worked well with the exception of sorting. If records had to be sorted by a translated field, than it did not work, because sorting happened only when the default language was fetched. This was always broken. I guess Kasper just did not see such use case.

Secondly, it seems that somebody decided to break this model. Whether it was conscious decision or that somebody just did not read the Frontend Localisation Guide, I do not know. In any case, TYPO3 got an option to add records in other languages without any relation to the parent record in the default language. Sometimes it is useful but it broke TYPO3 localisation completely. If people want to use such model, they should still use a record in the default language but hide it or make it empty (in case of tt_content). This already works with Kasper's model.

Now Extbase tried to work with both ways and it fails because these two ways are incompatible.

So we have several problems here:
  1. What to do with records that exist in non-default language only?
  2. Sorting of records using translated fields (like translated titles)?
I would solve the first problem as follows:
  1. Add a special option in query settings that will specially enable such behavior.
  2. If the field has l10n_mode set in $TCA, a default language record had to be fetched in some cases and proper overlays made.

This is quite complicated and slow.

For the second problem we may need some joins and more complex logic in Extbase. Again, l10n_mode can be a problem here.

There is no easy solution. I would even set the complexity to "nightmare" instead of "hard".

I am willing to be in a task force group for this. I may not have much time for coding (it depends on the amount of work I have at my daily job) but I would be happy to discuss, review and test the code after I am back from vacation.

#6 Updated by Dmitry Dulepov almost 3 years ago

I would fiully agree with the change at https://review.typo3.org/#/c/10188/ for now. But I would like to improve in other commits as described above.

#7 Updated by Helmut Hummel almost 3 years ago

  • Target version changed from Candidate for patchlevel to Candidate for Major Version
  • Complexity changed from hard to nightmare

This ticket is in line with all other tickets regarding language handling in general and language handling in Extbase.

Before we start "fixing" something, we need to define the behavior for every scenario and cover these with tests. Alex did start on that already: https://github.com/alexanderschnitzler/extbase_language_tests

I see no way to properly fix this in already released versions.

There is btw. a very simple workaround possible for this very issue: Create a DataHandler hook, that transfers the category relations to the translated records on save and update.

#8 Updated by Tymoteusz Motylewski over 1 year ago

  • Category changed from Extbase to Extbase + l10n

#9 Updated by Markus Mächler about 1 year ago

When using sys_language_mode=strict then the killer part looks like this:

AND (
    -- language set to all languages
    tx_news_domain_model_news.sys_language_uid = -1

    -- all translated records but without a language parent
    OR (
        tx_news_domain_model_news.sys_language_uid = 1 AND tx_news_domain_model_news.l10n_parent = 0

    -- records in the default language that have translations
    ) OR (
        tx_news_domain_model_news.sys_language_uid = 0 AND tx_news_domain_model_news.uid IN(
            SELECT tx_news_domain_model_news.l10n_parent
            FROM tx_news_domain_model_news
            WHERE tx_news_domain_model_news.l10n_parent > 0 AND tx_news_domain_model_news.sys_language_uid = 1 AND tx_news_domain_model_news.deleted = 0
        )
    )
)

That makes it impossible to search e.g. for a title of a properly translated news record because you either search records that have their language set to all, that have no language parent or are in the default language. I created a very limited fix for that issue, the fix does only work if all the fields that you want to search are explicitly set on the translated record: https://gist.github.com/maechler/258f06f170c7138500e1a3c4a9b12725

#10 Updated by Nicole Cordes about 1 year ago

Markus Mächler wrote:

That makes it impossible to search e.g. for a title of a properly translated news record because you either search records that have their language set to all, that have no language parent or are in the default language. I created a very limited fix for that issue, the fix does only work if all the fields that you want to search are explicitly set on the translated record: https://gist.github.com/maechler/258f06f170c7138500e1a3c4a9b12725

Hi Markus,

I think your message is not fully correct. The part `WHERE tx_news_domain_model_news.l10n_parent > 0 AND tx_news_domain_model_news.sys_language_uid = 1 AND tx_news_domain_model_news.deleted = 0` searches all entries in the current language (in your case sys_language_uid 1) that are translated. I don't see any issue here. This makes it perfectly working for your case: search by title in translated records.

Am I wrong?

Edit: I overlooked the outer condition. Indeed this needs to be removed to fetch correct results.
See https://review.typo3.org/37039/

#11 Updated by Markus Klein about 1 year ago

  • Target version deleted (Candidate for Major Version)
  • Complexity deleted (nightmare)

This needs to be tested again with #82363.

#12 Updated by Markus Klein about 1 year ago

  • Related to Bug #82363: Make Extbase translation handling consistent with typoscript added

#13 Updated by Markus Klein about 1 year ago

  • Related to Bug #63694: extbase: strict mode and bug with queries in regards addSysLanguageStatement - e.g. contains ignores different MM translations added

#14 Updated by Tymoteusz Motylewski 8 months ago

  • Status changed from Accepted to Needs Feedback

Please take a look if https://review.typo3.org/#/c/53974/ solves the issue for you.

#15 Updated by Markus Klein 8 months ago

  • Status changed from Needs Feedback to Closed

Continued with #82363.

Also available in: Atom PDF