[FIX] manifestEnhancer: Only use valid files for supportedLocales #1080
Conversation
lib/processors/manifestEnhancer.js
Outdated
| if (isValidPropertiesFilenameLocale(locale)) { | ||
| supportedLocales.push(locale); | ||
| } else { | ||
| log.verbose(`Skipping invalid locale '${locale}' for bundle '${i18nBundleUrl}'`); |
There was a problem hiding this comment.
Still open: What should happen in this case? Maybe rather an warning or even error log?
There was a problem hiding this comment.
Decision: Log a warning and explain in commit message that the warning can be suppressed by manually maintaining the "supportedLocales".
There was a problem hiding this comment.
To be checked: Built-in handling of private extensions at runtime (1Q, 2Q, 3Q).
There was a problem hiding this comment.
Warning added via 11ab19f
To be checked: Built-in handling of private extensions at runtime (1Q, 2Q, 3Q).
This just turned out to be a confusion based on a wrong comment / commit message. It referred to variants, but actually it was supposed to be extensions.
Extensions are not considered by the runtime code at all (i.e. can't appear in requests).
Private use is checked for the special SAP supportability locales, but the resulting request does not include an extension or private use section but will rather be e.g. en_US_saptrc (where saptrc is a variant).
matz3
force-pushed
the
fix-manifestEnhancer-invalid-locale
branch
2 times, most recently
from
eb9d5d2 to
11ab19f
Compare
matz3
force-pushed
the
fix-manifestEnhancer-invalid-locale
branch
from
b2fe975 to
9aea297
Compare
matz3
force-pushed
the
fix-manifestEnhancer-invalid-locale
branch
from
2c592a4 to
2745dba
Compare
matz3
force-pushed
the
fix-manifestEnhancer-invalid-locale
branch
from
b10b5d7 to
c8d83f1
Compare
There was a problem hiding this comment.
I'm fine with the code change. I struggled a bit wit hthe commit message. Would you mind detailing what "This is due to the fact that the ResourceBundle does use the legacy Java locale format for the request URL." means?
I guess you refer to the fact that not very possible part of a BCP47 language tag will be used by ResourceBundle for the outgoing requests (e.g. the script is currently ignored except for sr_Latn).
The references in the runtime code are meanwhile outdated (code moved to LanguageFallback), but they use a commit ID, so they continue to work.
This fixes two problems that could have occurred: A properties file with an invalid locale was still taken into the list of supported locales, which then caused a runtime exception in the ResourceBundle as it validates the input. Another problem was that properties files could have a valid name according to BCP47, but the file won't be ever requested with that name. This is due to the fact that the ResourceBundle does not use the extension / private use sections of the locale when creating the request URL. In both cases, the properties file is now ignored and no entry for the supportedLocales is created.
sapprc is not a valid supportability locale
Co-authored-by: Merlin Beutlberger <[email protected]>
matz3
force-pushed
the
fix-manifestEnhancer-invalid-locale
branch
from
c8d83f1 to
de919a1
Compare
With that I meant that the runtime never requests BCP47 language tags (i.e. dash-separated). It always uses the Java locale format (separated by underscores), isn't it? |
|
I didn't doubt that the comment is in general right (it is). It just took me a while to get what exactly this means. Adding some details or examples would have helped to understand the problem better. The difference between "_" and "-" is one such detail. The runtime also won't sent out certain subtags, e.g. the script (except for sr_Latn) and others (e.g. private extensions other than those for the pseudo languages). Well, but I don't want to overcomplicate it. |
|
@codeworrior thanks for your feedback. I've updated the commit message and also updated the code references. |
This fixes two problems that could have occurred:
A properties file with an invalid locale was still taken into the list of supported locales, which then caused a runtime exception in the ResourceBundle as it validates the input.
Another problem was that properties files could have a valid name according to BCP47, but the file won't be ever requested with that name.
This is due to the fact that the ResourceBundle does use the legacy Java locale format (using underscores instead of dashes) for the request URL.
In both cases, the properties file is now ignored and no entry for the supportedLocales is created.
Only locales that are valid according to the legacy Java locale format are considered.
However, there is one special case: sr_Latn is also requested by the UI5 runtime, although it contains a BCP47 script, which is not valid according to the legacy Java locale format.