1850787 - Investigate refactoring browser_shopping_onboarding to address intermittents and debug mode failures

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P2)

Comment 1

[Punam Dahiya [:pdahiya]]

Fix in patch https://phabricator.services.mozilla.com/D186272 adds dependency on productURL ( used to display site name in ftl string) to be available before displaying onboarding message. This fails browser/components/shopping/tests/browser/browser_shopping_onboarding.js test in debug mode on all platform with below error:

Fluent error, the argument "$currentSite" was not provided a value. [task 2023-08-29T21:39:30.683Z] 21:39:30 INFO - GECKO(25767) | This error happened while formatting the following messages: .... ."shopping-onboarding-headline" ... "shopping-onboarding-dynamic-subtitle" ... "shopping-onboarding-opt-in-privacy-policy-and-terms-of-use" ...Hit MOZ_CRASH(Resolver error: Unknown variable: $currentSite) at intl/l10n/rust/localization-ffi/src/lib.rs:620

https://treeherder.mozilla.org/jobs?repo=try&revision=74e79ad6f5a07649b85076d9b5cf13a9c6d293ac

Comment 2

[Punam Dahiya [:pdahiya]]

Tried with debug build on local, Opening about:shoppingsidebar crashes tab with below error:

Fluent error, the argument "$currentSite" was not provided a value.
0:24.12 GECKO(57816) This error happened while formatting the following messages:
0:24.12 GECKO(57816) "shopping-onboarding-headline"
0:24.12 GECKO(57816) "shopping-onboarding-dynamic-subtitle"
0:24.12 GECKO(57816) "shopping-onboarding-body"
0:24.12 GECKO(57816) "shopping-onboarding-opt-in-privacy-policy-and-terms-of-use"
0:24.12 GECKO(57816) "shopping-onboarding-opt-in-button"
0:24.12 GECKO(57816) "shopping-onboarding-not-now-button"
0:24.12 GECKO(57816) Hit MOZ_CRASH(Resolver error: Unknown variable: $currentSite) at intl/l10n/rust/localization-ffi/src/lib.rs:620
0:24.12 GECKO(57816) #01: RustMozCrash (~/mozglue/static/rust/wrappers.cpp:18)

https://searchfox.org/mozilla-central/source/intl/l10n/rust/localization-ffi/src/lib.rs#620

https://bugzilla.mozilla.org/show_bug.cgi?id=1685180

NI @gtatum for feedback and suggest fix thanks!

Comment 3

[Greg Tatum [:gregtatum] (REO Fx132)]

I haven't looked at the code, but my preferred way now is to not include the data-l10n-id on the element's markup, but instead select the element by id, and apply the data-l10n-id and arguments all at once. Generally this happens because the page is rendered before the arguments are provided. Instead, only apply the data-l10n-id when you have a known value in the argument.

Like this:

https://searchfox.org/mozilla-central/rev/c1fb133ca4901985070e519c92322b432fa254c5/browser/base/content/browser-places.js#235-244

A different work-around is to set currentSite to "" by default, but that's just masking the underlying issue.

Comment 4

[Emily McMinn :emcminn]

(In reply to Greg Tatum [:gregtatum] from comment #3)

I haven't looked at the code, but my preferred way now is to not include the data-l10n-id on the element's markup, but instead select the element by id, and apply the data-l10n-id and arguments all at once. Generally this happens because the page is rendered before the arguments are provided. Instead, only apply the data-l10n-id when you have a known value in the argument.

Like this:

https://searchfox.org/mozilla-central/rev/c1fb133ca4901985070e519c92322b432fa254c5/browser/base/content/browser-places.js#235-244

A different work-around is to set currentSite to "" by default, but that's just masking the underlying issue.

I think we can use this solution - thanks! I'll try it out :)

Comment 5

[Emily McMinn :emcminn]

Attachment: Bug 1850787 - Apply data-l10n-id & args simultaneously

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99d4b627061e
Apply data-l10n-id & args simultaneously r=omc-reviewers,aminomancer

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/99d4b627061e