diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index b910f58f5a17..c6eb6fca7aa3 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1293,7 +1293,7 @@ pref("browser.newtabpage.activity-stream.improvesearch.topSiteSearchShortcuts", // ASRouter provider configuration pref("browser.newtabpage.activity-stream.asrouter.providers.cfr", "{\"id\":\"cfr\",\"enabled\":true,\"type\":\"remote-settings\",\"bucket\":\"cfr\",\"frequency\":{\"custom\":[{\"period\":\"daily\",\"cap\":1}]},\"categories\":[\"cfrAddons\",\"cfrFeatures\"],\"updateCycleInMs\":3600000}"); pref("browser.newtabpage.activity-stream.asrouter.providers.whats-new-panel", "{\"id\":\"whats-new-panel\",\"enabled\":true,\"type\":\"remote-settings\",\"bucket\":\"whats-new-panel\",\"updateCycleInMs\":3600000}"); -pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "{\"id\":\"message-groups\",\"enabled\":true,\"type\":\"remote-settings\",\"bucket\":\"message-groups\",\"updateCycleInMs\":3600000}"); +pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "{\"id\":\"message-groups\",\"enabled\":false,\"type\":\"remote-settings\",\"bucket\":\"message-groups\",\"updateCycleInMs\":3600000}"); // This url, if changed, MUST continue to point to an https url. Pulling arbitrary content to inject into // this page over http opens us up to a man-in-the-middle attack that we'd rather not face. If you are a downstream // repackager of this code using an alternate snippet url, please keep your users safe diff --git a/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx b/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx index fd95d5697c02..76fbf49bfef3 100644 --- a/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx +++ b/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx @@ -1620,26 +1620,22 @@ export class ASRouterAdminInner extends React.PureComponent { Enabled Impressions count Custom frequency - User preferences {this.state.groups && - this.state.groups.map( - ({ id, enabled, frequency, userPreferences = [] }, index) => ( - - - - - {this._getGroupImpressionsCount(id, frequency)} - {JSON.stringify(frequency, null, 2)} - {userPreferences.join(", ")} - - ) - )} + this.state.groups.map(({ id, enabled, frequency }, index) => ( + + + + + {this._getGroupImpressionsCount(id, frequency)} + {JSON.stringify(frequency, null, 2)} + + ))} ); diff --git a/browser/components/newtab/data/content/activity-stream.bundle.js b/browser/components/newtab/data/content/activity-stream.bundle.js index 1e8cb0087d66..1c467f3f675f 100644 --- a/browser/components/newtab/data/content/activity-stream.bundle.js +++ b/browser/components/newtab/data/content/activity-stream.bundle.js @@ -1987,18 +1987,17 @@ class ASRouterAdminInner extends react__WEBPACK_IMPORTED_MODULE_4___default.a.Pu case "groups": return react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement(react__WEBPACK_IMPORTED_MODULE_4___default.a.Fragment, null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("h2", null, "Message Groups"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("table", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("thead", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("tr", { className: "message-item" - }, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "Enabled"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "Impressions count"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "Custom frequency"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "User preferences"))), this.state.groups && this.state.groups.map(({ + }, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "Enabled"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "Impressions count"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, "Custom frequency"))), this.state.groups && this.state.groups.map(({ id, enabled, - frequency, - userPreferences = [] + frequency }, index) => react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement(Row, { key: id }, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement(TogglePrefCheckbox, { checked: enabled, pref: id, onChange: this.toggleGroups - })), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, this._getGroupImpressionsCount(id, frequency)), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, JSON.stringify(frequency, null, 2)), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, userPreferences.join(", ")))))); + })), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, this._getGroupImpressionsCount(id, frequency)), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, JSON.stringify(frequency, null, 2)))))); case "ds": return react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement(react__WEBPACK_IMPORTED_MODULE_4___default.a.Fragment, null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("h2", null, "Discovery Stream"), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement(DiscoveryStreamAdmin, { diff --git a/browser/components/newtab/lib/ASRouter.jsm b/browser/components/newtab/lib/ASRouter.jsm index e8c2025883d3..c0dfe60c78de 100644 --- a/browser/components/newtab/lib/ASRouter.jsm +++ b/browser/components/newtab/lib/ASRouter.jsm @@ -972,6 +972,17 @@ class _ASRouter { (await this._storage.get("groupBlockList")) || [] ).concat(providerBlockList); + // Merge any existing provider impressions into the corresponding group + // Don't keep providerImpressions in state anymore + const providerImpressions = + (await this._storage.get("providerImpressions")) || {}; + for (const provider of Object.keys(providerImpressions)) { + groupImpressions[provider] = [ + ...(groupImpressions[provider] || []), + ...providerImpressions[provider], + ]; + } + const previousSessionEnd = (await this._storage.get("previousSessionEnd")) || 0; await this.setState({ @@ -1518,6 +1529,11 @@ class _ASRouter { state.groups, "groupImpressions" ); + this._cleanupImpressionsForItems( + state, + state.providers, + "providerImpressions" + ); return { messageImpressions, groupImpressions }; }); } @@ -1710,8 +1726,11 @@ class _ASRouter { await this.setState(state => { const providerBlockList = [...state.providerBlockList, ...idsToBlock]; + // When a provider is blocked, its impressions should be cleared as well + const providerImpressions = { ...state.providerImpressions }; + idsToBlock.forEach(id => delete providerImpressions[id]); this._storage.set("providerBlockList", providerBlockList); - return { providerBlockList }; + return { providerBlockList, providerImpressions }; }); } diff --git a/browser/components/newtab/test/browser/browser.ini b/browser/components/newtab/test/browser/browser.ini index ac8bd9f38680..ad1dd0945099 100644 --- a/browser/components/newtab/test/browser/browser.ini +++ b/browser/components/newtab/test/browser/browser.ini @@ -43,6 +43,4 @@ tags = remote-settings [browser_asrouter_momentspagehub.js] tags = remote-settings [browser_asrouter_experimentsAPILoader.js] -[browser_asrouter_group_frequency.js] -[browser_asrouter_group_userprefs.js] [browser_asrouter_trigger_docs.js] diff --git a/browser/components/newtab/test/browser/browser_asrouter_group_frequency.js b/browser/components/newtab/test/browser/browser_asrouter_group_frequency.js deleted file mode 100644 index 02120766b9d4..000000000000 --- a/browser/components/newtab/test/browser/browser_asrouter_group_frequency.js +++ /dev/null @@ -1,163 +0,0 @@ -const { ASRouter } = ChromeUtils.import( - "resource://activity-stream/lib/ASRouter.jsm" -); -const { RemoteSettings } = ChromeUtils.import( - "resource://services-settings/remote-settings.js" -); -const { CFRMessageProvider } = ChromeUtils.import( - "resource://activity-stream/lib/CFRMessageProvider.jsm" -); - -/** - * Load and modify a message for the test. - */ -add_task(async function setup() { - // Force the WNPanel provider cache to 0 by modifying updateCycleInMs - await SpecialPowers.pushPrefEnv({ - set: [ - [ - "browser.newtabpage.activity-stream.asrouter.providers.cfr", - `{"id":"cfr","enabled":true,"type":"remote-settings","bucket":"cfr","categories":["cfrAddons","cfrFeatures"],"updateCycleInMs":0}`, - ], - ], - }); - - const initialMsgCount = ASRouter.state.messages.length; - - const heartbeatMsg = CFRMessageProvider.getMessages().find( - m => m.id === "HEARTBEAT_TACTIC_2" - ); - const testMessage = { - ...heartbeatMsg, - groups: ["messaging-experiments"], - targeting: "true", - // Ensure no overlap due to frequency capping with other tests - id: `HEARTBEAT_MESSAGE_${Date.now()}`, - }; - const client = RemoteSettings("cfr"); - await client.db.clear(); - await client.db.create(testMessage); - await client.db.saveLastModified(42); // Prevent from loading JSON dump. - - // Reload the providers - await BrowserTestUtils.waitForCondition(async () => { - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - return ASRouter.state.messages.length > initialMsgCount; - }, "Should load the extra heartbeat message"); - - BrowserTestUtils.waitForCondition( - () => ASRouter.state.messages.find(m => m.id === testMessage.id), - "Wait to load the message" - ); - - const msg = ASRouter.state.messages.find(m => m.id === testMessage.id); - Assert.equal(msg.targeting, "true"); - Assert.equal(msg.groups[0], "messaging-experiments"); - - registerCleanupFunction(async () => { - await client.db.clear(); - // Reload the providers - await BrowserTestUtils.waitForCondition(async () => { - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - return ASRouter.state.messages.length === initialMsgCount; - }, "Should reset messages"); - await SpecialPowers.popPrefEnv(); - }); -}); - -/** - * Test group frequency capping. - * Message has a lifetime frequency of 3 but it's group has a lifetime frequency - * of 2. It should only show up twice. - * We update the provider to remove any daily limitations so it should show up - * on every new tab load. - */ -add_task(async function test_heartbeat_tactic_2() { - const TEST_URL = "http://example.com"; - - // Force the WNPanel provider cache to 0 by modifying updateCycleInMs - await SpecialPowers.pushPrefEnv({ - set: [ - [ - "browser.newtabpage.activity-stream.asrouter.providers.message-groups", - `{"id":"message-groups","enabled":true,"type":"remote-settings","bucket":"message-groups","updateCycleInMs":0}`, - ], - ], - }); - const groupConfiguration = { - id: "messaging-experiments", - enabled: true, - userPreferences: [], - frequency: { lifetime: 2 }, - }; - const client = RemoteSettings("message-groups"); - await client.db.clear(); - await client.db.create(groupConfiguration); - await client.db.saveLastModified(42); // Prevent from loading JSON dump. - - // Reload the providers - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - - await BrowserTestUtils.waitForCondition( - () => ASRouter.state.groups.find(g => g.id === groupConfiguration.id), - "Wait for group config to load" - ); - let groupState = ASRouter.state.groups.find( - g => g.id === groupConfiguration.id - ); - Assert.ok(groupState, "Group config found"); - Assert.ok(groupState.enabled, "Group is enabled"); - - let tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - await BrowserTestUtils.loadURI(tab1.linkedBrowser, TEST_URL); - - let chiclet = document.getElementById("contextual-feature-recommendation"); - Assert.ok(chiclet, "CFR chiclet element found (tab1)"); - await BrowserTestUtils.waitForCondition( - () => !chiclet.hidden, - "Chiclet should be visible (tab1)" - ); - - let tab2 = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - await BrowserTestUtils.loadURI(tab2.linkedBrowser, TEST_URL); - - Assert.ok(chiclet, "CFR chiclet element found (tab2)"); - await BrowserTestUtils.waitForCondition( - () => !chiclet.hidden, - "Chiclet should be visible (tab2)" - ); - - // Wait for the message to become blocked (frequency limit reached) - const msg = ASRouter.state.messages.find(m => - m.groups.includes("messaging-experiments") - ); - Assert.ok(msg, "Message found"); - await BrowserTestUtils.waitForCondition( - () => !ASRouter.isBelowFrequencyCaps(msg), - "Message frequency limit should be reached" - ); - - let tab3 = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - await BrowserTestUtils.loadURI(tab3.linkedBrowser, TEST_URL); - - await BrowserTestUtils.waitForCondition( - () => chiclet.hidden, - "Heartbeat button should be hidden" - ); - - info("Cleanup"); - BrowserTestUtils.removeTab(tab1); - BrowserTestUtils.removeTab(tab2); - BrowserTestUtils.removeTab(tab3); - await client.db.clear(); - // Reset group impressions - await ASRouter.setGroupState({ id: "messaging-experiments", value: true }); - await ASRouter.setGroupState({ id: "cfr", value: true }); - // Reload the providers - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - await SpecialPowers.popPrefEnv(); -}); diff --git a/browser/components/newtab/test/browser/browser_asrouter_group_userprefs.js b/browser/components/newtab/test/browser/browser_asrouter_group_userprefs.js deleted file mode 100644 index ffea86ef6bb1..000000000000 --- a/browser/components/newtab/test/browser/browser_asrouter_group_userprefs.js +++ /dev/null @@ -1,143 +0,0 @@ -const { ASRouter } = ChromeUtils.import( - "resource://activity-stream/lib/ASRouter.jsm" -); -const { RemoteSettings } = ChromeUtils.import( - "resource://services-settings/remote-settings.js" -); -const { CFRMessageProvider } = ChromeUtils.import( - "resource://activity-stream/lib/CFRMessageProvider.jsm" -); - -/** - * Load and modify a message for the test. - */ -add_task(async function setup() { - // Force the WNPanel provider cache to 0 by modifying updateCycleInMs - await SpecialPowers.pushPrefEnv({ - set: [ - [ - "browser.newtabpage.activity-stream.asrouter.providers.cfr", - `{"id":"cfr","enabled":true,"type":"remote-settings","bucket":"cfr","updateCycleInMs":0}`, - ], - ], - }); - - const initialMsgCount = ASRouter.state.messages.length; - - const heartbeatMsg = CFRMessageProvider.getMessages().find( - m => m.id === "HEARTBEAT_TACTIC_2" - ); - const testMessage = { - ...heartbeatMsg, - groups: ["messaging-experiments"], - targeting: "true", - // Ensure no overlap due to frequency capping with other tests - id: `HEARTBEAT_MESSAGE_${Date.now()}`, - }; - const client = RemoteSettings("cfr"); - await client.db.clear(); - await client.db.create(testMessage); - await client.db.saveLastModified(42); // Prevent from loading JSON dump. - - // Reload the providers - await BrowserTestUtils.waitForCondition(async () => { - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - return ASRouter.state.messages.length > initialMsgCount; - }, "Should load the extra heartbeat message"); - - BrowserTestUtils.waitForCondition( - () => ASRouter.state.messages.find(m => m.id === testMessage.id), - "Wait to load the message" - ); - - const msg = ASRouter.state.messages.find(m => m.id === testMessage.id); - Assert.equal(msg.targeting, "true"); - Assert.equal(msg.groups[0], "messaging-experiments"); - Assert.ok(ASRouter.isUnblockedMessage(msg), "Message is unblocked"); - - registerCleanupFunction(async () => { - await client.db.clear(); - // Reload the providers - await BrowserTestUtils.waitForCondition(async () => { - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - return ASRouter.state.messages.length === initialMsgCount; - }, "Should reset messages"); - await SpecialPowers.popPrefEnv(); - }); -}); - -/** - * Test group user preferences. - * Group is enabled if both user preferences are enabled. - */ -add_task(async function test_heartbeat_tactic_2() { - const TEST_URL = "http://example.com"; - - await SpecialPowers.pushPrefEnv({ - set: [ - [ - "browser.newtabpage.activity-stream.asrouter.providers.message-groups", - `{"id":"message-groups","enabled":true,"type":"remote-settings","bucket":"message-groups","updateCycleInMs":0}`, - ], - ], - }); - const groupConfiguration = { - id: "messaging-experiments", - enabled: true, - userPreferences: ["browser.userPreference.messaging-experiments"], - }; - const client = RemoteSettings("message-groups"); - await client.db.clear(); - await client.db.create(groupConfiguration); - await client.db.saveLastModified(42); // Prevent from loading JSON dump. - - // Reload the providers - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - - await BrowserTestUtils.waitForCondition( - () => ASRouter.state.groups.find(g => g.id === groupConfiguration.id), - "Wait for group config to load" - ); - let groupState = ASRouter.state.groups.find( - g => g.id === groupConfiguration.id - ); - Assert.ok(groupState, "Group config found"); - Assert.ok(groupState.enabled, "Group is enabled"); - - let tab1 = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - await BrowserTestUtils.loadURI(tab1.linkedBrowser, TEST_URL); - - let chiclet = document.getElementById("contextual-feature-recommendation"); - Assert.ok(chiclet, "CFR chiclet element found"); - await BrowserTestUtils.waitForCondition( - () => !chiclet.hidden, - "Chiclet should be visible (userprefs enabled)" - ); - - await SpecialPowers.pushPrefEnv({ - set: [["browser.userPreference.messaging-experiments", false]], - }); - - let tab2 = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL); - await BrowserTestUtils.loadURI(tab2.linkedBrowser, TEST_URL); - - await BrowserTestUtils.waitForCondition( - () => chiclet.hidden, - "Heartbeat button should not be visible (userprefs disabled)" - ); - - info("Cleanup"); - BrowserTestUtils.removeTab(tab1); - BrowserTestUtils.removeTab(tab2); - await client.db.clear(); - // Reset group impressions - await ASRouter.setGroupState({ id: "messaging-experiments", value: true }); - await ASRouter.setGroupState({ id: "cfr", value: true }); - // Reload the providers - await ASRouter._updateMessageProviders(); - await ASRouter.loadMessagesFromAllProviders(); - await SpecialPowers.popPrefEnv(); -}); diff --git a/testing/profiles/common/user.js b/testing/profiles/common/user.js index 0f0e5ad85519..112a4c5807b7 100644 --- a/testing/profiles/common/user.js +++ b/testing/profiles/common/user.js @@ -9,9 +9,6 @@ user_pref("devtools.console.stdout.chrome", true); user_pref("browser.newtabpage.activity-stream.asrouter.providers.cfr", "[]"); user_pref("browser.newtabpage.activity-stream.asrouter.providers.cfr-fxa", "[]"); user_pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "[]"); -user_pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "[]"); -user_pref("browser.newtabpage.activity-stream.asrouter.providers.whats-new-panel", "[]"); -user_pref("browser.newtabpage.activity-stream.asrouter.providers.messaging-experiments", "[]"); user_pref("browser.newtabpage.activity-stream.feeds.section.topstories", false); user_pref("browser.newtabpage.activity-stream.feeds.snippets", false); user_pref("browser.newtabpage.activity-stream.tippyTop.service.endpoint", "");