diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index f20cfc020505..898cf66d85f6 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1282,6 +1282,7 @@ pref("browser.newtabpage.activity-stream.asrouter.providers.message-groups", "{\ // 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 pref("browser.newtabpage.activity-stream.asrouter.providers.snippets", "{\"id\":\"snippets\",\"enabled\":true,\"type\":\"remote\",\"url\":\"https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/\",\"updateCycleInMs\":14400000}"); +pref("browser.newtabpage.activity-stream.asrouter.providers.snippets-preview", "{\"id\":\"snippets-preview\",\"enabled\":true,\"type\":\"remote\",\"url\":\"\",\"updateCycleInMs\":0}"); #ifdef NIGHTLY_BUILD pref("browser.newtabpage.activity-stream.asrouter.useReleaseSnippets", true); #endif diff --git a/browser/components/newtab/lib/ASRouter.jsm b/browser/components/newtab/lib/ASRouter.jsm index 7b58a814d0fc..ba17a93602ec 100644 --- a/browser/components/newtab/lib/ASRouter.jsm +++ b/browser/components/newtab/lib/ASRouter.jsm @@ -68,11 +68,12 @@ const TRAILHEAD_CONFIG = { const INCOMING_MESSAGE_NAME = "ASRouter:child-to-parent"; const OUTGOING_MESSAGE_NAME = "ASRouter:parent-to-child"; const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000; +const PREVIEW_SNIPPETS_ID = "snippets-preview"; // List of hosts for endpoints that serve router messages. // Key is allowed host, value is a name for the endpoint host. const DEFAULT_WHITELIST_HOSTS = { "activity-stream-icons.services.mozilla.com": "production", - "snippets-admin.mozilla.org": "preview", + "snippets-admin.mozilla.org": PREVIEW_SNIPPETS_ID, }; const SNIPPETS_ENDPOINT_WHITELIST = "browser.newtab.activity-stream.asrouter.whitelistHosts"; @@ -603,7 +604,7 @@ class _ASRouter { const previousProviders = this.state.providers; const providers = [ // If we have added a `preview` provider, hold onto it - ...previousProviders.filter(p => p.id === "preview"), + ...previousProviders.filter(p => p.id === PREVIEW_SNIPPETS_ID), // The provider should be enabled and not have a user preference set to false ...ASRouterPreferences.providers.filter( p => @@ -698,6 +699,9 @@ class _ASRouter { * @returns bool */ hasGroupsEnabled(groups = []) { + if (groups.includes(PREVIEW_SNIPPETS_ID)) { + return true; + } return this.state.groups .filter(({ id }) => groups.includes(id)) .every(({ enabled }) => enabled); @@ -710,10 +714,7 @@ class _ASRouter { */ isExcludedByProvider(message) { const provider = this.state.providers.find(p => p.id === message.provider); - if (!provider) { - return true; - } - if (provider.exclude) { + if (provider && provider.exclude) { return provider.exclude.includes(message.id); } return false; @@ -849,7 +850,7 @@ class _ASRouter { } // We don't want to cache preview endpoints, remove them after messages are fetched - await this.setState(this._removePreviewEndpoint(newState)); + await this.setState(this._disablePreviewEndpoint(newState)); await this.cleanupImpressions(); } } @@ -1757,7 +1758,7 @@ class _ASRouter { // `preview` so that the updateCycle is 0 return additionalHosts.reduce( (whitelist_hosts, host) => { - whitelist_hosts[host] = "preview"; + whitelist_hosts[host] = PREVIEW_SNIPPETS_ID; Services.console.logStringMessage(`Adding ${host} to whitelist hosts.`); return whitelist_hosts; }, @@ -1777,27 +1778,38 @@ class _ASRouter { }); } - _removePreviewEndpoint(state) { - state.providers = state.providers.filter(p => p.id !== "preview"); + _disablePreviewEndpoint(state) { + const previewProvider = this.state.providers.find( + p => p.id === PREVIEW_SNIPPETS_ID + ); + if (previewProvider && previewProvider.enabled) { + const providers = this.state.providers.filter( + p => p.id !== PREVIEW_SNIPPETS_ID + ); + previewProvider.url = ""; + previewProvider.enabled = false; + providers.push(previewProvider); + state.providers = providers; + } + return state; } async _addPreviewEndpoint(url, portID) { - // When you view a preview snippet we want to hide all real content - const providers = [...this.state.providers]; - if ( - this._validPreviewEndpoint(url) && - !providers.find(p => p.url === url) - ) { + const providers = [...this.state.providers].filter( + p => p.id !== PREVIEW_SNIPPETS_ID + ); + const previewProvider = this.state.providers.find( + p => p.id === PREVIEW_SNIPPETS_ID + ); + if (this._validPreviewEndpoint(url)) { this.dispatchToAS( + // When you view a preview snippet we want to hide all real content ac.OnlyToOneContent({ type: at.SNIPPETS_PREVIEW_MODE }, portID) ); - providers.push({ - id: "preview", - type: "remote", - url, - updateCycleInMs: 0, - }); + previewProvider.enabled = true; + previewProvider.url = url; + providers.push(previewProvider); await this.setState({ providers }); } } @@ -2020,7 +2032,9 @@ class _ASRouter { await this.loadMessagesFromAllProviders(); if (endpoint) { - message = await this.handleMessageRequest({ provider: "preview" }); + message = await this.handleMessageRequest({ + provider: PREVIEW_SNIPPETS_ID, + }); // We don't want to cache preview messages, remove them after we selected the message to show if (message) { @@ -2168,12 +2182,7 @@ class _ASRouter { await this.setMessageById(action.data.id, target, true, action); break; case "ADMIN_CONNECT_STATE": - if (action.data && action.data.endpoint) { - this._addPreviewEndpoint(action.data.endpoint.url, target.portID); - await this.loadMessagesFromAllProviders(); - } else { - await this._updateAdminState(target); - } + await this._updateAdminState(target); break; case "IMPRESSION": await this.addImpression(action.data); diff --git a/browser/components/newtab/test/browser/browser.ini b/browser/components/newtab/test/browser/browser.ini index 00ee5ef2311d..c6390a3281d6 100644 --- a/browser/components/newtab/test/browser/browser.ini +++ b/browser/components/newtab/test/browser/browser.ini @@ -3,6 +3,7 @@ support-files = blue_page.html red_page.html head.js + snippet.json prefs = browser.newtabpage.activity-stream.debug=false browser.newtabpage.activity-stream.discoverystream.enabled=true diff --git a/browser/components/newtab/test/browser/browser_asrouter_snippets.js b/browser/components/newtab/test/browser/browser_asrouter_snippets.js index 6e1ce54b5cb8..265364ce893a 100644 --- a/browser/components/newtab/test/browser/browser_asrouter_snippets.js +++ b/browser/components/newtab/test/browser/browser_asrouter_snippets.js @@ -64,3 +64,38 @@ test_newtab({ ); }, }); + +add_task(async () => { + ASRouter._validPreviewEndpoint = () => true; + await BrowserTestUtils.withNewTab( + { + gBrowser, + url: + "about:newtab?endpoint=https://example.com/browser/browser/components/newtab/test/browser/snippet.json", + }, + async browser => { + let text = await SpecialPowers.spawn(browser, [], async () => { + await ContentTaskUtils.waitForCondition( + () => content.document.querySelector(".activity-stream"), + `Should render Activity Stream` + ); + await ContentTaskUtils.waitForCondition( + () => + content.document.querySelector( + "#footer-asrouter-container .SimpleSnippet" + ), + "Should find the snippet inside the footer container" + ); + + return content.document.querySelector("#footer-asrouter-container") + .innerText; + }); + + Assert.equal( + text, + "On January 30th Nightly will introduce dedicated profiles, making it simpler to run different installations of Firefox side by side. Learn what this means for you.", + "Snippet content match" + ); + } + ); +}); diff --git a/browser/components/newtab/test/browser/snippet.json b/browser/components/newtab/test/browser/snippet.json new file mode 100644 index 000000000000..7b92288b0fc1 --- /dev/null +++ b/browser/components/newtab/test/browser/snippet.json @@ -0,0 +1,25 @@ +{ + "messages": [ + { + "weight": 50, + "id": "10533", + "template": "simple_snippet", + "template_version": "1.0.0", + "content": { + "icon": "", + "text": "On January 30th Nightly will introduce dedicated profiles, making it simpler to run different installations of Firefox side by side. Learn what this means for you.", + "tall": false, + "do_not_autoblock": false, + "links": { + "link0": { + "url": "https://support.mozilla.org/kb/dedicated-profiles-firefox-installation/?utm_source=desktop-snippet&utm_medium=snippet&utm_campaign=nightly_profile_management&utm_term=10533&utm_content=nightly" + } + } + }, + "campaign": "nightly-profile-management", + "targeting": "true", + "provider_url": "https://snippets.cdn.mozilla.net/6/Firefox/66.0a1/20190122215349/Darwin_x86_64-gcc3/en-US/default/Darwin%2018.0.0/default/default/", + "provider": "snippets" + } + ] +} diff --git a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js index 5019651cfce2..48bc7ff248a2 100644 --- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js +++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js @@ -50,6 +50,7 @@ function fakeExecuteUserAction(action) { return fakeAsyncMessage({ data: action, type: "USER_ACTION" }); } +// eslint-disable-next-line max-statements describe("ASRouter", () => { let Router; let globals; @@ -345,7 +346,11 @@ describe("ASRouter", () => { getStringPrefStub.returns('["whitelist.com"]'); await createRouterAndInit(); - assert.propertyVal(Router.WHITELIST_HOSTS, "whitelist.com", "preview"); + assert.propertyVal( + Router.WHITELIST_HOSTS, + "whitelist.com", + "snippets-preview" + ); // Should still include the defaults assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 3); }); @@ -357,7 +362,7 @@ describe("ASRouter", () => { assert.propertyVal( Router.WHITELIST_HOSTS, "snippets-admin.mozilla.org", - "preview" + "snippets-preview" ); assert.propertyVal( Router.WHITELIST_HOSTS, @@ -1156,6 +1161,28 @@ describe("ASRouter", () => { }); assert.isNull(result); }); + it("should allow messages with undefined providers (like 'preview')", async () => { + sandbox.stub(Router, "loadMessagesFromAllProviders"); + await Router.setState(() => ({ + providers: [{ id: "snippets", exclude: ["foo"] }], + })); + + await Router.setState(() => ({ + messages: [ + { + id: "foo", + provider: "snippets-preview", + groups: ["snippets-preview"], + }, + ], + messageBlockList: ["foocampaign"], + })); + + const result = await Router.handleMessageRequest({ + provider: "snippets-preview", + }); + assert.propertyVal(result, "provider", "snippets-preview"); + }); it("should not return a message if the frequency cap has been hit", async () => { sandbox.stub(Router, "isBelowFrequencyCaps").returns(false); await Router.setState(() => ({ @@ -1636,6 +1663,18 @@ describe("ASRouter", () => { }); }); describe("#_addPreviewEndpoint", () => { + beforeEach(async () => { + await Router.setState(state => { + const providers = [...state.providers]; + const snippetsPreview = { + id: "snippets-preview", + enabled: false, + type: "remote", + }; + providers.push(snippetsPreview); + return { providers }; + }); + }); it("should make a request to the provided endpoint on NEWTAB_MESSAGE_REQUEST", async () => { const url = "https://snippets-admin.mozilla.org/foo"; const msg = fakeAsyncMessage({ @@ -1647,17 +1686,6 @@ describe("ASRouter", () => { assert.calledWith(global.fetch, url); assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0); }); - it("should make a request to the provided endpoint on ADMIN_CONNECT_STATE and remove the endpoint", async () => { - const url = "https://snippets-admin.mozilla.org/foo"; - const msg = fakeAsyncMessage({ - type: "ADMIN_CONNECT_STATE", - data: { endpoint: { url } }, - }); - await Router.onMessage(msg); - - assert.calledWith(global.fetch, url); - assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0); - }); it("should dispatch SNIPPETS_PREVIEW_MODE when adding a preview endpoint", async () => { const url = "https://snippets-admin.mozilla.org/foo"; const msg = fakeAsyncMessage({ @@ -2013,12 +2041,12 @@ describe("ASRouter", () => { it("should return the preview message if that's available and remove it from Router.state", async () => { const expectedObj = { id: "foo", - groups: ["preview"], - provider: "preview", + groups: ["snippets-preview"], + provider: "snippets-preview", }; Router.setState({ messages: [expectedObj], - providers: [{ id: "preview" }], + providers: [{ id: "snippets-preview" }], }); await Router.sendNewTabMessage(channel, { endpoint: "foo.com" }); @@ -2029,7 +2057,7 @@ describe("ASRouter", () => { { type: "SET_MESSAGE", data: expectedObj } ); assert.isUndefined( - Router.state.messages.find(m => m.provider === "preview") + Router.state.messages.find(m => m.provider === "snippets-preview") ); }); it("should call _getBundledMessages if we request a message that needs to be bundled", async () => { @@ -3519,6 +3547,23 @@ describe("ASRouter", () => { assert.notCalled(Router._storage.set); assert.deepEqual(Router.state.messageImpressions, messageImpressions); }); + it("should call setState before calling cleanupImpressions", async () => { + // The call order in `loadMessagesFromAllProviders` is important + const messages = [ + { id: "foo", frequency: { lifetime: 10 } }, + { id: "bar", frequency: { lifetime: 10 } }, + ]; + const setStateStub = sandbox.stub(Router, "setState"); + const cleanupImpressionsStub = sandbox.stub( + Router, + "cleanupImpressions" + ); + await createRouterAndInit([ + { id: "onboarding", type: "local", messages, enabled: true }, + ]); + + cleanupImpressionsStub.calledImmediatelyAfter(setStateStub); + }); }); }); @@ -4038,4 +4083,25 @@ describe("ASRouter", () => { }); }); }); + describe("#loadMessagesForProvider", () => { + it("should fetch json from url", async () => { + let result = await MessageLoaderUtils.loadMessagesForProvider({ + location: "http://fake.com/endpoint", + type: "json", + }); + + assert.property(result, "messages"); + assert.lengthOf(result.messages, FAKE_REMOTE_MESSAGES.length); + }); + it("should catch errors", async () => { + fetchStub.throws(); + let result = await MessageLoaderUtils.loadMessagesForProvider({ + location: "http://fake.com/endpoint", + type: "json", + }); + + assert.property(result, "messages"); + assert.lengthOf(result.messages, 0); + }); + }); }); diff --git a/testing/profiles/common/user.js b/testing/profiles/common/user.js index 8f7fbdd31f21..03632dfb08b6 100644 --- a/testing/profiles/common/user.js +++ b/testing/profiles/common/user.js @@ -9,6 +9,7 @@ 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.snippets-preview", "[]"); 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", "");