BUGFIX: Restore XLIFF translation fallback #1094
Conversation
|
Thanks, this makes sense! Left two comments. Also, this should probably be targeted lower (3.3 even as it's LTS and still in bugfix support). |
| if (is_dir($translationPath)) { | ||
| $this->readDirectoryRecursively($translationPath, $parsedData, $fileId, $package->getPackageKey()); | ||
| $localeChain = $this->localizationService->getLocaleChain($locale); | ||
| foreach (array_reverse($localeChain) as $localeChainItem) { |
There was a problem hiding this comment.
It's not so obvious why the locale chain needs to be walked in reverse, can we maybe add a small explanatory comment?
| @@ -87,26 +94,29 @@ public function initializeObject() | |||
| */ | |||
| public function getMergedFileData($fileId, Locale $locale): array | |||
| { | |||
| if (!isset($this->files[$fileId][$locale->getLanguage()])) { | |||
| if (!isset($this->files[$fileId][$locale->__toString()])) { | |||
There was a problem hiding this comment.
Inside the localization service we use (string)$locale instead. Would be nice to have that in line
|
Thanks! Can you also rebase this on 3.3, since simply changing the base won't work in this case. |
|
This file is new in 4.2, so we can't rebase this to 3.3. Before 4.2, Flow used a different approach in loading XLIFF files (see #894) |
|
Ah yes, of course! My bad :) Then this just needs a rebase on 4.2, which should be pretty easy. |
| @@ -87,26 +94,30 @@ public function initializeObject() | |||
| */ | |||
| public function getMergedFileData($fileId, Locale $locale): array | |||
| { | |||
| if (!isset($this->files[$fileId][$locale->getLanguage()])) { | |||
There was a problem hiding this comment.
hmm feels wrong to me to rely on the magic __toString() method, maybe add a getIdentifier or getLocaleIdentifier method and use that instead? __toString() could do that same then of course..
but maybe that's just me..
There was a problem hiding this comment.
I'd say a getIdentifier() method in \Neos\Flow\I18n\Locale is generally a good thing, but it's also a medium change to Flow if you consequently replace each Locale string cast with the new method. This is just a humble bugfix ;)
There was a problem hiding this comment.
you're right, seems like it's used a bunch of other places in the code so guess someone meant to use that way.. nvm then
fritjofbohm
force-pushed
the
fix-xliff-language-fallback
branch
from
045b175
to
1cef43d
Compare
|
OK, I did a rebase of this stuff on 4.2. Please let me know if I did it right. |
|
Seems a couple of unrelated commits sneaked in. Try pulling latest 4.2 branch, then (force) rebase in interactive mode on that and skip all but your two commits (if there are still any other). |
This modification fixes the translation fallback mechanism and the ability to read regionalized language variants. Issues fixed: * Translation fallback did not work. `Neos.Flow.i18n.fallbackRule.order` was ignored when looking for XLIFF files * When using a regional `Neos.Flow.i18n.defaultLocale` (e.g. `de_AT` for an Austrian version of German `de`), XLIFF files were never read from a directory with the full `defaultLocale` name, but only from the base language directory. In the example, files in `Packages/Application/Vendor.Package/Resources/Private/Translations/de_AT` were never read, only `Packages/Application/Vendor.Package/Resources/Private/Translations/de`.
… and add comment * `$locale` is now cast the way it is in `\Neos\Flow\I18n\Service` * Added a small comment why the locale chain is reversed
fritjofbohm
force-pushed
the
fix-xliff-language-fallback
branch
from
1cef43d
to
2f0a48c
Compare
|
Next try. Better this time? |
…back BUGFIX: Restore XLIFF translation fallback This modification fixes the translation fallback mechanism and the ability to read regionalized language variants. Issues fixed: * Translation fallback did not work. `Neos.Flow.i18n.fallbackRule.order` was ignored when looking for XLIFF files * When using a regional `Neos.Flow.i18n.defaultLocale` (e.g. `de_AT` for an Austrian version of `de`), XLIFF files were never read from a directory with the full `defaultLocale` name, but only from the base language directory. In the example, files in `Packages/Application/Vendor.Package/Resources/Private/Translations/de_AT` were never read, only from`Packages/Application/Vendor.Package/Resources/Private/Translations/de`.
This modification fixes the translation fallback mechanism and the ability to read regionalized language variants.
Issues fixed:
Neos.Flow.i18n.fallbackRule.orderwas ignored when looking for XLIFF filesNeos.Flow.i18n.defaultLocale(e.g.de_ATfor an Austrian version ofde), XLIFF files were never read from a directory with the fulldefaultLocalename, but only from the base language directory. In the example, files inPackages/Application/Vendor.Package/Resources/Private/Translations/de_ATwere never read, only fromPackages/Application/Vendor.Package/Resources/Private/Translations/de.