ezpublish-legacy icon indicating copy to clipboard operation
ezpublish-legacy copied to clipboard

Fix translation for some search messages when using ezfind

Open edorfaus opened this issue 12 years ago • 3 comments

I noticed that when using eZ Find with eZ Publish 4.6, some of the messages on the search page are not localized, including the main "Search" header.

This is because the eZ Find extension includes those messages in its translation.ts files, just marked with type="unfinished". As mentioned here, this used to work just fine, making this a regression in later versions. It was introduced by commit cb0878b3b00aa393f060dce9d4905c4c04f346b4, which makes sure unfinished translations are cached but also made them overwrite the actual translations from the base translation files.

This patch fixes the problem by not replacing existing translations with ones that are marked unfinished, while leaving the behaviour unchanged for all other cases.

edorfaus avatar Dec 18 '12 16:12 edorfaus

As far as I can tell, the Travis build failure is unrelated to this patch.

Of the two builds, one failed because the git clone took too long, the other because of a test regarding expiry of PHP cache for class identifiers, which has nothing to do with the code this patch touches (as far as I can tell anyway).

edorfaus avatar Dec 18 '12 17:12 edorfaus

I have the same problem with eg. ezwebin extension, which removes existing translations by their empty declaration. About your patch: What about the situation where all translation "instances" are unfinished? Will it be added as an unfinished? Is the order of translations taken into account (extension order)?

trolek avatar Dec 19 '12 07:12 trolek

Yeah, the problem (and the fix in the patch) is not limited to ezfind, that's just where I found it - the PR title was just the best one I could think of that would fit.

This patch does not change the load order (which I assume is the same as the extension order).

Basically, the way this code works is that it takes each translation.ts file for the appropriate locale (first the base one, then from each enabled translation extension), and for each of the files, it loads the messages found in it into an in-memory cache (which is later saved to disk for faster reload in subsequent requests).

The cache is basically a hash array, keyed by the context, comment and source message, and is used to look up the individual translation strings when they are needed.

Originally, before the commit I mentioned, each message was checked for having a non-empty translation string before being added into the cache. Since unfinished strings are usually left empty, this meant that they would not replace any translations that were already in the cache, but it also meant that messages with no translations would not be put into the cache in the first place.

I guess there was some kind of performance impact by not finding a message in the cache, as the above-mentioned commit changed this so that the message without a translation is put into the cache using the source string as the translated string. If the message is not translated anywhere else, that is basically what you would have gotten anyway - so I guess the author of that commit simply didn't think of the case we're seeing now.

With my patch, the only change is that it looks for the type="unfinished" attribute on the translation, and if that is found, it does not replace an existing translation that is already in the cache, but if this is the first time this message is seen, it is still added.

So, if a message is defined in the base (or an earlier extension), unfinished translations in later extensions will not replace them, but finished ones will. Similarly, if one extension defines a new message, but leaves it unfinished, a later extension can replace this unfinished translation with a finished one - and even if it's left unfinished, it is still added into the cache like the above commit wanted.

Note, however, that if all translation "instances" are unfinished, it is the first one that will be used - normally, this is not a problem, as all of them are empty anyway, but if you're doing something odd with this, you may need to revisit how that works in your case (though you'd probably have to do that for the above commit anyway).

edorfaus avatar Dec 19 '12 10:12 edorfaus