1814318 - The weather suggestion UI shows out-of-date info

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

This is a regression due to the l10n cache change in bug 1811373. The weather UI ends up using out-of-date cached strings for the current temperature and high/low text. For example if the UI shows 20° as the current temperature and Firefox fetches a new suggestion with 19° as the current temperature, the UI will keep showing 20°.

By design, L10nCache.ensure() doesn't recache a string if it's already cached. The problem here is that we set excludeArgsFromCacheKey to true, which means as long as the string is cached with any argument value, then ensure() won't recache it. If the argument value changes -- e.g., from 20 to 19 -- too bad.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1811373

Comment 2

[Drew Willcoxon :adw]

Attachment: Bug 1814318 - Make L10nCache.ensure() re-cache strings when excludeArgsFromCacheKey is set.

This is an error in how I implemented the excludeArgsFromCacheKey mechanism in
D167318. It can be fixed by always re-caching strings when
excludeArgsFromCacheKey is passed to ensure(). We could instead store the
argument values of currently cached strings and then re-cache only when
different argument values are passed to ensure(), but that would require a
deeper change and I don't think it's worth it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6bc7d03059ad
Make L10nCache.ensure() re-cache strings when excludeArgsFromCacheKey is set. r=dao

Comment 4

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/6bc7d03059ad

Comment 5

[Carmen Fat [:cfat] - Ecosystem QA]

@Drew, In order to verify this issue and make sure that the temperature values are correctly updated, would it be enough to compare the Weather suggestion values to the values displayed by the Accuweather website and confirm they are the same? Or do you have a more accurate idea on how to verify this issue?

Comment 6

[Drew Willcoxon :adw]

This will be hard to verify in a realistic way. The bug happens when the temperature stored in the fetched suggestion doesn't match the one that's shown. That means you'll need to wait until the temperature actually changes, verify the fetched suggestion has the new temperature, and then make sure Firefox shows the new temperature. I don't think you need to worry about all that.

Even with the bug fixed, it's still possible that the temperature Firefox shows will be slightly different from the one on AccuWeather's site, due to the fact that Firefox only fetches the suggestion every 30 minutes.

You can try verifying it this way:

1. Start Firefox and enable the weather feature
2. Click in the urlbar and verify a weather suggestion is shown
3. Close the urlbar panel
4. Run the JS below in the browser console. It sets the C and F temperatures to 100 in the fetched suggestion
5. Click in the urlbar again and verify the temperature under "Currently" is 100

(function() {
let { QuickSuggest } = ChromeUtils.importESModule(
  "resource:///modules/QuickSuggest.sys.mjs"
);
QuickSuggest.weather.suggestion.current_conditions.temperature.c = 100;
QuickSuggest.weather.suggestion.current_conditions.temperature.f = 100;
console.log(`ok`, QuickSuggest.weather.suggestion);
})();

Comment 7

[Carmen Fat [:cfat] - Ecosystem QA]

Thank you Drew for the detailed explanation and the workaround. Using the steps you provided above I was able to verify this issue on Firefox Release 111.0.1 (Build ID: 20230321111920) on Firefox Beta 112 RC (Build ID: 20230403163424) and the latest Nightly 113.0a1 (Build ID: 20230404134922) on Windows 10 x64, macOS 12.6.1 and Linux Ubuntu 20.04 x64.

• The current temperature from the weather result is updated correctly based on the values contained by the snippet from comment 6.