Closed Bug 1642725 Opened 4 years ago Closed 4 years ago

aboutNetError triggers full l10n fallback chain just to check if a string exists

Categories

(Firefox :: Security, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- wontfix
firefox80 --- fixed

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.

Regressed by: 1622269
Has Regression Range: --- → yes

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.

The severity field is not set for this bug.
:johannh, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jhofmann)
Severity: -- → S3
Flags: needinfo?(gijskruitbosch+bugs)
Priority: -- → P3
Flags: needinfo?(jhofmann)
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f40a3d5e3f30 use sets of known message identifiers to limit possible error titles and messages instead of consulting fluent, r=zbraniecki,prathiksha,fluent-reviewers,Pike
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 80
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: