The weather suggestion UI shows out-of-date info
Categories
(Firefox :: Address Bar, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr102 | --- | unaffected |
firefox109 | --- | unaffected |
firefox110 | --- | unaffected |
firefox111 | --- | verified |
firefox112 | --- | verified |
firefox113 | --- | verified |
People
(Reporter: adw, Assigned: adw)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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•2 years ago
|
||
Set release status flags based on info from the regressing bug 1811373
Assignee | ||
Comment 2•2 years ago
|
||
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.
Assignee | ||
Updated•2 years ago
|
Comment 4•2 years ago
|
||
bugherder |
Comment 5•2 years ago
|
||
@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?
Assignee | ||
Comment 6•2 years ago
|
||
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:
- Start Firefox and enable the weather feature
- Click in the urlbar and verify a weather suggestion is shown
- Close the urlbar panel
- Run the JS below in the browser console. It sets the C and F temperatures to 100 in the fetched suggestion
- 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•2 years ago
|
||
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.
Description
•