From ab5f6e6216c41d15ca77e5e1dcc0c270428068a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Fri, 24 Jul 2020 14:13:08 +0000 Subject: [PATCH] Bug 1653932 - Add pref for top site defaults from remote settings. r=mikedeboer Differential Revision: https://phabricator.services.mozilla.com/D84532 --- browser/app/profile/firefox.js | 2 + browser/components/newtab/lib/Screenshots.jsm | 10 ++- .../components/newtab/lib/TopSitesFeed.jsm | 82 ++++++++++++++----- 3 files changed, 73 insertions(+), 21 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 3a33c1cc4b1b..3da916c48905 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1291,6 +1291,8 @@ pref("prompts.tab_modal.enabled", true); // This is a fallback value for when prompt callers do not specify a modalType. pref("prompts.defaultModalType", 3); +pref("browser.topsites.useRemoteSetting", false); + // Activates preloading of the new tab url. pref("browser.newtab.preload", true); diff --git a/browser/components/newtab/lib/Screenshots.jsm b/browser/components/newtab/lib/Screenshots.jsm index 3b17956755ff..b58737e2b35d 100644 --- a/browser/components/newtab/lib/Screenshots.jsm +++ b/browser/components/newtab/lib/Screenshots.jsm @@ -121,9 +121,17 @@ this.Screenshots = { if (!this._shouldGetScreenshots()) { return; } + // __sharedCache may not exist yet for links from default top sites that + // don't have a default tippy top icon. + // eslint-disable-next-line no-unsanitized/property + link.__sharedCache ??= { + updateLink(prop, val) { + link[prop] = val; + }, + }; + const cache = link.__sharedCache; // Nothing to do if we already have a pending screenshot or // if a previous request failed and returned null. - const cache = link.__sharedCache; if (cache.fetchingScreenshot || link[property] !== undefined) { return; } diff --git a/browser/components/newtab/lib/TopSitesFeed.jsm b/browser/components/newtab/lib/TopSitesFeed.jsm index 400af043cf6a..fb8543dfaf4f 100644 --- a/browser/components/newtab/lib/TopSitesFeed.jsm +++ b/browser/components/newtab/lib/TopSitesFeed.jsm @@ -94,6 +94,9 @@ for (let searchProvider of ["amazon", "google"]) { ); } +const REMOTE_SETTING_DEFAULTS_PREF = "browser.topsites.useRemoteSetting"; +const REMOTE_SETTING_OVERRIDE_PREF = "browser.topsites.default"; + function getShortURLForCurrentSearch() { const url = shortURL({ url: Services.search.defaultEngine.searchForm }); return url; @@ -125,15 +128,15 @@ this.TopSitesFeed = class TopSitesFeed { init() { // If the feed was previously disabled PREFS_INITIAL_VALUES was never received - this.refreshDefaults( - this.store.getState().Prefs.values[DEFAULT_SITES_PREF] - ); + this._readDefaults(); this._storage = this.store.dbStorage.getDbTable("sectionPrefs"); this.refresh({ broadcast: true, isStartup: true }); Services.obs.addObserver(this, "browser-search-engine-modified"); for (let [pref] of SEARCH_TILE_OVERRIDE_PREFS) { Services.prefs.addObserver(pref, this); } + Services.prefs.addObserver(REMOTE_SETTING_DEFAULTS_PREF, this); + Services.prefs.addObserver(REMOTE_SETTING_OVERRIDE_PREF, this); } uninit() { @@ -142,24 +145,35 @@ this.TopSitesFeed = class TopSitesFeed { for (let [pref] of SEARCH_TILE_OVERRIDE_PREFS) { Services.prefs.removeObserver(pref, this); } + Services.prefs.removeObserver(REMOTE_SETTING_DEFAULTS_PREF, this); + Services.prefs.removeObserver(REMOTE_SETTING_OVERRIDE_PREF, this); } observe(subj, topic, data) { - // We should update the current top sites if the search engine has been changed since - // the search engine that gets filtered out of top sites has changed. - if ( - topic === "browser-search-engine-modified" && - data === "engine-default" && - this.store.getState().Prefs.values[FILTER_DEFAULT_SEARCH_PREF] - ) { - delete this._currentSearchHostname; - this._currentSearchHostname = getShortURLForCurrentSearch(); - this.refresh({ broadcast: true }); - } else if ( - topic === "nsPref:changed" && - SEARCH_TILE_OVERRIDE_PREFS.has(data) - ) { - this.refresh({ broadcast: true }); + switch (topic) { + case "browser-search-engine-modified": + // We should update the current top sites if the search engine has been changed since + // the search engine that gets filtered out of top sites has changed. + if ( + data === "engine-default" && + this.store.getState().Prefs.values[FILTER_DEFAULT_SEARCH_PREF] + ) { + delete this._currentSearchHostname; + this._currentSearchHostname = getShortURLForCurrentSearch(); + this.refresh({ broadcast: true }); + } + break; + case "nsPref:changed": + if ( + data === REMOTE_SETTING_DEFAULTS_PREF || + data === REMOTE_SETTING_OVERRIDE_PREF + ) { + this._readDefaults(); + this.refresh({ broadcast: true }); + } else if (SEARCH_TILE_OVERRIDE_PREFS.has(data)) { + this.refresh({ broadcast: true }); + } + break; } } @@ -167,6 +181,30 @@ this.TopSitesFeed = class TopSitesFeed { return site && site.hostname; } + /** + * _readDefaults - sets DEFAULT_TOP_SITES + */ + _readDefaults() { + this._useRemoteSetting = Services.prefs.getBoolPref( + REMOTE_SETTING_DEFAULTS_PREF + ); + + if (!this._useRemoteSetting) { + this.refreshDefaults( + this.store.getState().Prefs.values[DEFAULT_SITES_PREF] + ); + return; + } + + let sites; + try { + sites = Services.prefs.getStringPref(REMOTE_SETTING_OVERRIDE_PREF); + } catch (e) {} + // Placeholder for the actual remote setting (bug 1653937). + sites ??= "https://mozilla.org,https://firefox.com"; + this.refreshDefaults(sites); + } + refreshDefaults(sites) { // Clear out the array of any previous defaults DEFAULT_TOP_SITES.length = 0; @@ -884,7 +922,9 @@ this.TopSitesFeed = class TopSitesFeed { case at.PREF_CHANGED: switch (action.data.name) { case DEFAULT_SITES_PREF: - this.refreshDefaults(action.data.value); + if (!this._useRemoteSetting) { + this.refreshDefaults(action.data.value); + } break; case ROWS_PREF: case FILTER_DEFAULT_SEARCH_PREF: @@ -906,7 +946,9 @@ this.TopSitesFeed = class TopSitesFeed { } break; case at.PREFS_INITIAL_VALUES: - this.refreshDefaults(action.data[DEFAULT_SITES_PREF]); + if (!this._useRemoteSetting) { + this.refreshDefaults(action.data[DEFAULT_SITES_PREF]); + } break; case at.TOP_SITES_PIN: this.pin(action);