Avoid translating empty values when using events

[Teddytrombone]

It's great we can use the CanFieldBeTranslatedCheckEvent to change if a field can be translated. The problem though is that the event doesn't know anything about the field's value.
For example: we use a custom renderType for previewing SERPs for fields like seo_title or description which leads to dd_deepl not to translate the description field of pages. So we created an event, which enables this. Of course we could have used translateWithDeepl in this case, but this is not always applicable.
So as we use the event, we force the translation for all description fields, even for pages of type 'folder'. But the description there is null which leads to an error, as DeeplTranslationService->translateFieldInternal expects a string for $fieldValue.

I would suggest to add the fieldValue to the event (and maybe some information why the result has the value it has?) or make some sanity checks so that even if overridden by an event no empty/null values are passed.

[Teddytrombone]

A suggestion to this: maybe change canFieldBeTranslated to only check if a field can be translated based on the TCA configuration, not on the value. So if after the canFieldBeTranslated returns true, make an independent check if the value should be transmitted to DeepL. For this check - independent on the TCA - do strip tags, decode html entities, trim etc. the value and if it's empty or only contains numbers or content, which don't need to be translated, send it to the API.
Because - although this might be edge cases - on one hand input fields and textfields without RTE enabled can contain HTML code and on the other hand RTE fields can only contain numbers 😉

[dmitryd]

@Teddytrombone

Because - although this might be edge cases - on one hand input fields and textfields without RTE enabled can contain HTML code and on the other hand RTE fields can only contain numbers

You can exclude any field from translation via TCA. You can set $GLOBALS['TCA']['tx_xyz']['columns'][$fieldName]['config']['translateWithDeepl'] to false.

For RTE with numbers - yes, may be it should be checked too...

[Teddytrombone]

If I set translateWithDeepl to false other checks of the if statements are made and then the field is possibly not excluded when a condition is met. The result is forced to true if translateWithDeepl is not false, but it's not possible to force it to false.

Have a look at my PR #23: i changed the if statement to check, if translateWithDeepl is set and then sets the result to the value of this setting.

[dmitryd]

I did it slightly differently. If translateWithDeepl is set then:

• if false, then the field is never translated
• if true, then other checks run

[Teddytrombone]

I thought the intention of translateWithDeepl was to be able to force a vertain behaviour of a field. So the use should be intentional. But I understand that you want to have the other checks also executed.
The problem we have now is, that because of the check for softref in input fields we cannot send tt_content's subheader field to DeepL (we did this by setting translateWithDeepl to true). But I guess we can use the event for this or remove the softref in our projects 😉
So feel free to close my PR and this Issue 😀

[dmitryd]

The option is not documented yet, so we can change the behavior. The main idea was to prohibit translating some fields, like map coordinates or map addresses or person names.

I can make it to bypass other checks but still call the event. Would this work for you?

[Teddytrombone]

Yes, I think that would work. As I suggested earlier maybe it would be a good idea to separate the checks into kind of TCA based checks and content based checks? So that e.g. empty values aren't transmitted, even if a field is forced for translation?

[dmitryd]

@Teddytrombone Could you check if this commit helps you to achive what you want?