See bug 1856014 for a bunch of issues that went uncaught because they are just warnings. It also appears that the warnings only make it to the console in debug builds. I don't really understand why, off-hand - perhaps there's a race condition, perhaps something else - the code in https://searchfox.org/mozilla-central/rev/2f5b657343ce18e4b8a56417f707022550f4b742/dom/l10n/DOMLocalization.cpp#579 or its callers don't seem obviously debug-specific ? Though it also claims to be mostly not hit by instrumented builds, which perhaps is a different kind of clue about what is going wrong. I don't know.

I looked at mozilla-central linux debug logs and asked the structured log viewer to look for fluent-dom and it found quite a few other ones besides 1856014. As a basic assumption I believe they are likely all errors of some kind, so if/when we make them fatal we may initially need an allowlist so that we catch new instances, while creating time for various frontend/core teams to fix the existing ones.

The logging of these is errors debug-only by adding them only in #ifdef DEBUG blocks: https://searchfox.org/mozilla-central/source/dom/l10n/L10nOverlays.cpp#423,506

I'd be a bit hesitant about making tests fail for these errors, as there are legitimate use cases of this behaviour as a feature. For example, the data-l10n-id of an element could change from a message which includes an inner element to one which does not. Should that really be forbidden?

(In reply to Eemeli Aro [:eemeli] from comment #1)

The logging of these is errors debug-only by adding them only in #ifdef DEBUG blocks: https://searchfox.org/mozilla-central/source/dom/l10n/L10nOverlays.cpp#423,506

Ah, thanks for clarifying.

I'd be a bit hesitant about making tests fail for these errors, as there are legitimate use cases of this behaviour as a feature. For example, the data-l10n-id of an element could change from a message which includes an inner element to one which does not. Should that really be forbidden?

If that shouldn't be an error, why are we treating it like one and logging it in debug? :-)

More seriously, I think the case you outline is probably OK and should not generate errors. However, the cases I'm seeing (certainly 100% of the ones in bug 1856014) are people specifying fluent messages in a way that does not interact correctly with the (custom/lit) element being localized (but happens to more or less kind of work which may mean they don't notice). AIUI fluent should be able to tell the difference because it knows whether we're in an initial translation or not, and/or whether there was a (different) l10n id on the element beforehand. Right?

(In reply to :Gijs (he/him) from comment #2)

More seriously, I think the case you outline is probably OK and should not generate errors. However, the cases I'm seeing (certainly 100% of the ones in bug 1856014) are people specifying fluent messages in a way that does not interact correctly with the (custom/lit) element being localized (but happens to more or less kind of work which may mean they don't notice). AIUI fluent should be able to tell the difference because it knows whether we're in an initial translation or not, and/or whether there was a (different) l10n id on the element beforehand. Right?

Sort of. In an initial translation DocumentL10n::TranslateDocument calls DOMLocalization::TranslateElements, which calls DOMLocalization::ApplyTranslations, which calls L10nOverlays::TranslateElement, which detects the error. So the is-initial-translation info would need to get passed through all of those in order to effect the error handling.

Capturing the had-previous-l10n-id info would probably need a bit more wrangling, as that would need to get detected in DOMLocalization::SetAttributes and stored somewhere so that it's accessible from L10nOverlays::TranslateElement.

To me, this all seems like more work than is perhaps worthwhile.

(In reply to Eemeli Aro [:eemeli] from comment #3)

(In reply to :Gijs (he/him) from comment #2)

More seriously, I think the case you outline is probably OK and should not generate errors. However, the cases I'm seeing (certainly 100% of the ones in bug 1856014) are people specifying fluent messages in a way that does not interact correctly with the (custom/lit) element being localized (but happens to more or less kind of work which may mean they don't notice). AIUI fluent should be able to tell the difference because it knows whether we're in an initial translation or not, and/or whether there was a (different) l10n id on the element beforehand. Right?

Sort of. In an initial translation DocumentL10n::TranslateDocument calls DOMLocalization::TranslateElements, which calls DOMLocalization::ApplyTranslations, which calls L10nOverlays::TranslateElement, which detects the error. So the is-initial-translation info would need to get passed through all of those in order to effect the error handling.

Or we could expose the state through the DOMLocalization or DocumentL10n object while the translation is happening, right?

Capturing the had-previous-l10n-id info would probably need a bit more wrangling, as that would need to get detected in DOMLocalization::SetAttributes and stored somewhere so that it's accessible from L10nOverlays::TranslateElement.

That does sound messy.

To me, this all seems like more work than is perhaps worthwhile.

I don't know that I agree. It's clear the cost of fixing this bug is not some kind of one-line/trivial fix, which I assume is why you think it's not worthwhile.

From my PoV as a desktop Firefox reviewer, the point of these errors is to stop developers from making mistakes. This type of mistake is almost impossible to stop using static analysis or linting (because the link between the FTL file and the file in which the message gets used goes through 2-4 layers of abstractions). It's also hard for developers or reviewers to notice in automated testing on a per-feature basis (as we don't test the outcome of l10n other than checking appropriate l10n IDs are set, if we even do that). It's also difficult to spot when reviewing (because the outcome looks superficially correct and so does the code). The warnings seem aimed to help with this, but currently 0 frontend developers see them (we pretty much never use debug builds), so they might as well not exist.

To me, the question is how often do these messages happen and do they signify a problem, ie what value are we missing because the messages are invisible. Searching all the Windows browser mochitest debug logs for fluent-dom and uniq'ifying the results yields:

An element named "icon" wasn't found in the source.
The element using message "extension-controlled-enable" was removed from the DOM when translating its "description" parent.
While translating an element with fluent ID "appmenuitem-fxa-manage-account" a child element of type "label" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-accept-option-label-2" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-1" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-2" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-3" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-4" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-5" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-6" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-7" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefox-suggest-onboarding-main-description-8" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefoxview-dont-remember-history-empty-description" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefoxview-history-empty-description" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "firefoxview-recentlyclosed-empty-description" a child element of type "a" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "ion-document-title" a child element of type "p" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "ion-no-current-studies" a child element of type "div" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "ion-summary" a child element of type "p" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "network-proxy-connection-description" a child element of type "strong" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "protections-panel-cookie-banner-view-cancel" a child element of type "hbox" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "protections-panel-cookie-banner-view-turn-off" a child element of type "hbox" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "protections-panel-cookie-banner-view-turn-on" a child element of type "hbox" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "protections-panel-not-blocking-why-etp-off-tooltip" a child element of type "description" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "protections-panel-not-blocking-why-etp-on-tooltip" a child element of type "description" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.
While translating an element with fluent ID "remove-search-engine-button" a child element of type "hbox" was removed. Either the fluent message does not contain markup, or it does not contain markup of this type.

I think that's quite a lot. But maybe that's just me? The fxview ones look like they might be of the variation you described (ie we swap a message with a link for one without a link). Less so the other ones inasmuch as I've checked.

Flod, any thoughts on this?

How realistic would it be to try making a decision based on the current data? Meaning, how much work would go into that investigation?

1. Check logs for errors (that's comment 4).
2. File separate bugs to have developers investigate the errors for their components, to decide if the warning is expected, or it's an actual error in the code. Also, does it create unexpected behavior?
3. Make a decision based on how many of these turn out to be expected.

I think we had a case recently where unrelated code was creating the issue (bug 1848707), and the initial attempted fix was terrible (adding an empty anchor in the string to ensure the element would always exist).

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

For more information, please visit BugBot documentation.