Closed Bug 1685729 Opened 8 months ago Closed 8 months ago

Refresh top sites on region change

Categories

(Firefox :: Top Sites, enhancement, P1)

enhancement
Points:
2

Tracking

()

VERIFIED FIXED
86 Branch
Iteration:
86.3 - Jan 11 - Jan 24
Tracking Status
firefox-esr78 --- unaffected
firefox84 --- wontfix
firefox85 --- wontfix
firefox86 --- verified

People

(Reporter: mikedeboer, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

When Region.jsm notifies the region-updated message, which happens when the user's home location changes, TopSitesFeed.jsm should reload the list of (default) Top Sites, because they're filtered on region (usually).

It's not absolutely necessary for all open New Tab pages to pick up this change right away. If it cheap enough, great!

Blocks: 1653929
Severity: -- → N/A
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Points: --- → 2
Summary: Change TopSitesFeed to observe the region-updated notification → Refresh top sites on region change
Iteration: --- → 86.3 - Jan 11 - Jan 24
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ccbeb11eca6f
Refresh top sites on region change. r=daleharvey
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

This fix is testable using a snippet that forces the region change:

var {Region} = ChromeUtils.import("resource://gre/modules/Region.jsm")
Region._setHomeRegion("RU")

Discussing with :dharvey and :standard8, the region change happens as soon as the region change timer hits - which might be in the middle of the session. When the region change hits, the search-config also triggers the search engine update based on the updated region.

For the Topsites with RS on and the fix -Nightly 86 (Mac/Windows) - most of the time, the region change will not trigger the topsite update for subsequent newly open about:newtab or about:home: it needs restart for the Topsites to pick the region change. However, we have had some small amout of tests in which the region update has triggered the Topsites update as well in the session (for subsequent new tabs), but we're unsure about the pre-requisites to hit that scenario.

Flags: qe-verify+
Flags: needinfo?(dao+bmo)

Sigh. Turns out the notification is called "browser-region", not "region-updated". "region-updated" is only added as extra data and basically redundant. (IMHO, the notification being called "browser-region-updated" and data being null would have been more in line with how notifications are normally set up.)

(In reply to Dão Gottwald [::dao] from comment #6)

Created attachment 9198617 [details]
Bug 1685729 - Use correct notification name for region changes. r=daleharvey

Sigh. Turns out the notification is called "browser-region", not "region-updated". "region-updated" is only added as extra data and basically redundant. (IMHO, the notification being called "browser-region-updated" and data being null would have been more in line with how notifications are normally set up.)

FWIW, feel free to change it while it's hot! There's no consumer for this notification atm, so should be relatively straightforward.

(In reply to Mike de Boer [:mikedeboer] from comment #7)

Sigh. Turns out the notification is called "browser-region", not "region-updated". "region-updated" is only added as extra data and basically redundant. (IMHO, the notification being called "browser-region-updated" and data being null would have been more in line with how notifications are normally set up.)

FWIW, feel free to change it while it's hot! There's no consumer for this notification atm, so should be relatively straightforward.

Filed bug 1688220.

Blocks: 1688220
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9755a79772b5
Use correct notification name for region changes. r=Standard8

Verified as fixed on Ubuntu 20.04, Mac 10.13.6 and Windows 10 using 86.0a1 (2021-01-25).
In regards to verification details, the tests were done using comment 3 snippet for region change and browser.topsites.useRemoteSetting -true.

As a functionality sumup, with the fix, topsites will now update and pick up the update on region change in all new tabs. (topsites with RS - on)

Status: RESOLVED → VERIFIED
Flags: qe-verify+ → in-qa-testsuite?(adrian.florinescu)
You need to log in before you can comment on or make changes to this bug.