diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 898cf66d85f6..f20cfc020505 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1282,7 +1282,6 @@ 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 ba17a93602ec..7b58a814d0fc 100644 --- a/browser/components/newtab/lib/ASRouter.jsm +++ b/browser/components/newtab/lib/ASRouter.jsm @@ -68,12 +68,11 @@ 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_ID, + "snippets-admin.mozilla.org": "preview", }; const SNIPPETS_ENDPOINT_WHITELIST = "browser.newtab.activity-stream.asrouter.whitelistHosts"; @@ -604,7 +603,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_SNIPPETS_ID), + ...previousProviders.filter(p => p.id === "preview"), // The provider should be enabled and not have a user preference set to false ...ASRouterPreferences.providers.filter( p => @@ -699,9 +698,6 @@ 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); @@ -714,7 +710,10 @@ class _ASRouter { */ isExcludedByProvider(message) { const provider = this.state.providers.find(p => p.id === message.provider); - if (provider && provider.exclude) { + if (!provider) { + return true; + } + if (provider.exclude) { return provider.exclude.includes(message.id); } return false; @@ -850,7 +849,7 @@ class _ASRouter { } // We don't want to cache preview endpoints, remove them after messages are fetched - await this.setState(this._disablePreviewEndpoint(newState)); + await this.setState(this._removePreviewEndpoint(newState)); await this.cleanupImpressions(); } } @@ -1758,7 +1757,7 @@ class _ASRouter { // `preview` so that the updateCycle is 0 return additionalHosts.reduce( (whitelist_hosts, host) => { - whitelist_hosts[host] = PREVIEW_SNIPPETS_ID; + whitelist_hosts[host] = "preview"; Services.console.logStringMessage(`Adding ${host} to whitelist hosts.`); return whitelist_hosts; }, @@ -1778,38 +1777,27 @@ class _ASRouter { }); } - _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; - } - + _removePreviewEndpoint(state) { + state.providers = state.providers.filter(p => p.id !== "preview"); return state; } async _addPreviewEndpoint(url, portID) { - 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)) { + // 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) + ) { this.dispatchToAS( - // When you view a preview snippet we want to hide all real content ac.OnlyToOneContent({ type: at.SNIPPETS_PREVIEW_MODE }, portID) ); - previewProvider.enabled = true; - previewProvider.url = url; - providers.push(previewProvider); + providers.push({ + id: "preview", + type: "remote", + url, + updateCycleInMs: 0, + }); await this.setState({ providers }); } } @@ -2032,9 +2020,7 @@ class _ASRouter { await this.loadMessagesFromAllProviders(); if (endpoint) { - message = await this.handleMessageRequest({ - provider: PREVIEW_SNIPPETS_ID, - }); + message = await this.handleMessageRequest({ provider: "preview" }); // We don't want to cache preview messages, remove them after we selected the message to show if (message) { @@ -2182,7 +2168,12 @@ class _ASRouter { await this.setMessageById(action.data.id, target, true, action); break; case "ADMIN_CONNECT_STATE": - await this._updateAdminState(target); + if (action.data && action.data.endpoint) { + this._addPreviewEndpoint(action.data.endpoint.url, target.portID); + await this.loadMessagesFromAllProviders(); + } else { + 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 c6390a3281d6..00ee5ef2311d 100644 --- a/browser/components/newtab/test/browser/browser.ini +++ b/browser/components/newtab/test/browser/browser.ini @@ -3,7 +3,6 @@ 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 265364ce893a..6e1ce54b5cb8 100644 --- a/browser/components/newtab/test/browser/browser_asrouter_snippets.js +++ b/browser/components/newtab/test/browser/browser_asrouter_snippets.js @@ -64,38 +64,3 @@ 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 deleted file mode 100644 index 7b92288b0fc1..000000000000 --- a/browser/components/newtab/test/browser/snippet.json +++ /dev/null @@ -1,25 +0,0 @@ -{ - "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 48bc7ff248a2..5019651cfce2 100644 --- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js +++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js @@ -50,7 +50,6 @@ function fakeExecuteUserAction(action) { return fakeAsyncMessage({ data: action, type: "USER_ACTION" }); } -// eslint-disable-next-line max-statements describe("ASRouter", () => { let Router; let globals; @@ -346,11 +345,7 @@ describe("ASRouter", () => { getStringPrefStub.returns('["whitelist.com"]'); await createRouterAndInit(); - assert.propertyVal( - Router.WHITELIST_HOSTS, - "whitelist.com", - "snippets-preview" - ); + assert.propertyVal(Router.WHITELIST_HOSTS, "whitelist.com", "preview"); // Should still include the defaults assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 3); }); @@ -362,7 +357,7 @@ describe("ASRouter", () => { assert.propertyVal( Router.WHITELIST_HOSTS, "snippets-admin.mozilla.org", - "snippets-preview" + "preview" ); assert.propertyVal( Router.WHITELIST_HOSTS, @@ -1161,28 +1156,6 @@ 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(() => ({ @@ -1663,18 +1636,6 @@ 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({ @@ -1686,6 +1647,17 @@ 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({ @@ -2041,12 +2013,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: ["snippets-preview"], - provider: "snippets-preview", + groups: ["preview"], + provider: "preview", }; Router.setState({ messages: [expectedObj], - providers: [{ id: "snippets-preview" }], + providers: [{ id: "preview" }], }); await Router.sendNewTabMessage(channel, { endpoint: "foo.com" }); @@ -2057,7 +2029,7 @@ describe("ASRouter", () => { { type: "SET_MESSAGE", data: expectedObj } ); assert.isUndefined( - Router.state.messages.find(m => m.provider === "snippets-preview") + Router.state.messages.find(m => m.provider === "preview") ); }); it("should call _getBundledMessages if we request a message that needs to be bundled", async () => { @@ -3547,23 +3519,6 @@ 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); - }); }); }); @@ -4083,25 +4038,4 @@ 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 03632dfb08b6..8f7fbdd31f21 100644 --- a/testing/profiles/common/user.js +++ b/testing/profiles/common/user.js @@ -9,7 +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.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", "");