aboutNetError triggers full l10n fallback chain just to check if a string exists
Categories
(Firefox :: Security, defect, P3)
Tracking
()
People
(Reporter: zbraniecki, Assigned: Gijs)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
Regressed by bug 1622269.
The issue is in https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/browser/base/content/aboutNetError.js#180-191
This code calls asynchronous localization method to check if the string with a given ID exists in the codebase.
This is an anti-pattern, because Fluent has a sophisticated lazy mechanisms to react to missing strings. In particular, it triggers loading of the fallback locale. In today's world in practice that may not seem like a lot, but as we improve our multilingualism, we'll get users with longer chain of locales that will all be loaded one by one just to verify that this string doesn't exist in any of them.
Instead, this code should have a list of known IDs and just check against it.
Updated•4 years ago
|
Reporter | ||
Comment 1•4 years ago
|
||
There's another place in this file where the same thing is used: https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/browser/base/content/aboutNetError.js#452-459
But this time, we need to call formatValues
because we actually use the result message to pass it to another message as an argument.
This should still benefit from the hardcoded list of errors because of the reasons mentioned above, and because in both cases the performance for the "generic-title" would be faster. Finally, it'll help us with linters for unused messages, because we need to know which strings are references anywhere in the code.
In the future, we hope to also support dynamic references, so this case will be encoded as:
document.l10n.setAttributes(desc, "ssl-connection-error", {
errorMessage: FluentMessage(l10nId),
hostname: hostString,
});
and Fluent will resolve l10nId
's value while resolving ssl-connection-error
.
For now, this bug is actionable by replacing both checks with a list of IDs to check against.
Updated•4 years ago
|
Comment 2•4 years ago
|
||
The severity field is not set for this bug.
:johannh, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Comment 5•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Description
•