☠☠ backed out by d6b6d63e601d ☠ ☠ | |
author | Scott <scott.downe@gmail.com> |
Mon, 25 May 2020 19:11:48 +0000 | |
changeset 532112 | dd35edffc6dffb7ae6a4050b955c6e7855cdffcd |
parent 532111 | da2c7b0ac9a44e44000a6be1cea9d753c33784d4 |
child 532113 | 2383139c85c0a60504a871530d099f96eff9e59d |
push id | 117001 |
push user | sdowne@getpocket.com |
push date | Mon, 25 May 2020 23:15:54 +0000 |
treeherder | autoland@dd35edffc6df [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | gvn, k88hudson |
bugs | 1446276 |
milestone | 78.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/components/newtab/content-src/components/Base/Base.jsx +++ b/browser/components/newtab/content-src/components/Base/Base.jsx @@ -127,17 +127,18 @@ export class BaseContent extends React.P const { App } = props; const { initialized } = App; const prefs = props.Prefs.values; const isDiscoveryStream = props.DiscoveryStream.config && props.DiscoveryStream.config.enabled; let filteredSections = props.Sections; - const pocketEnabled = prefs["feeds.section.topstories"]; + const pocketEnabled = + prefs["feeds.section.topstories"] && prefs["feeds.system.topstories"]; const noSectionsEnabled = !prefs["feeds.topsites"] && filteredSections.filter(section => section.enabled).length === 0; const searchHandoffEnabled = prefs["improvesearch.handoffToAwesomebar"]; const outerClassName = [ "outer-wrapper", isDiscoveryStream && pocketEnabled && "ds-outer-wrapper-search-alignment",
--- a/browser/components/newtab/content-src/lib/selectLayoutRender.js +++ b/browser/components/newtab/content-src/lib/selectLayoutRender.js @@ -69,17 +69,19 @@ export const selectLayoutRender = ({ if (!prefs["feeds.topsites"]) { filterArray.push("TopSites"); } if (!locale.startsWith("en-")) { filterArray.push("Navigation"); } - if (!prefs["feeds.section.topstories"]) { + const pocketEnabled = + prefs["feeds.section.topstories"] && prefs["feeds.system.topstories"]; + if (!pocketEnabled) { filterArray.push(...DS_COMPONENTS); } const placeholderComponent = component => { if (!component.feed) { // TODO we now need a placeholder for topsites and textPromo. return { ...component,
--- a/browser/components/newtab/data/content/activity-stream.bundle.js +++ b/browser/components/newtab/data/content/activity-stream.bundle.js @@ -738,17 +738,17 @@ class BaseContent extends react__WEBPACK App } = props; const { initialized } = App; const prefs = props.Prefs.values; const isDiscoveryStream = props.DiscoveryStream.config && props.DiscoveryStream.config.enabled; let filteredSections = props.Sections; - const pocketEnabled = prefs["feeds.section.topstories"]; + const pocketEnabled = prefs["feeds.section.topstories"] && prefs["feeds.system.topstories"]; const noSectionsEnabled = !prefs["feeds.topsites"] && filteredSections.filter(section => section.enabled).length === 0; const searchHandoffEnabled = prefs["improvesearch.handoffToAwesomebar"]; const outerClassName = ["outer-wrapper", isDiscoveryStream && pocketEnabled && "ds-outer-wrapper-search-alignment", isDiscoveryStream && "ds-outer-wrapper-breakpoint-override", prefs.showSearch && this.state.fixedSearch && !noSectionsEnabled && "fixed-search", prefs.showSearch && noSectionsEnabled && "only-search"].filter(v => v).join(" "); return react__WEBPACK_IMPORTED_MODULE_7___default.a.createElement("div", null, react__WEBPACK_IMPORTED_MODULE_7___default.a.createElement("div", { className: outerClassName }, react__WEBPACK_IMPORTED_MODULE_7___default.a.createElement("main", null, prefs.showSearch && react__WEBPACK_IMPORTED_MODULE_7___default.a.createElement("div", { className: "non-collapsible-section" }, react__WEBPACK_IMPORTED_MODULE_7___default.a.createElement(content_src_components_ErrorBoundary_ErrorBoundary__WEBPACK_IMPORTED_MODULE_6__["ErrorBoundary"], null, react__WEBPACK_IMPORTED_MODULE_7___default.a.createElement(content_src_components_Search_Search__WEBPACK_IMPORTED_MODULE_8__["Search"], _extends({ @@ -10704,17 +10704,19 @@ const selectLayoutRender = ({ if (!prefs["feeds.topsites"]) { filterArray.push("TopSites"); } if (!locale.startsWith("en-")) { filterArray.push("Navigation"); } - if (!prefs["feeds.section.topstories"]) { + const pocketEnabled = prefs["feeds.section.topstories"] && prefs["feeds.system.topstories"]; + + if (!pocketEnabled) { filterArray.push(...DS_COMPONENTS); } const placeholderComponent = component => { if (!component.feed) { // TODO we now need a placeholder for topsites and textPromo. return { ...component, data: {
--- a/browser/components/newtab/lib/ActivityStream.jsm +++ b/browser/components/newtab/lib/ActivityStream.jsm @@ -168,17 +168,17 @@ const PREFS_CONFIG = new Map([ "feeds.section.topstories.options", { title: "Configuration options for top stories feed", // This is a dynamic pref as it depends on the feed being shown or not getValue: args => JSON.stringify({ api_key_pref: "extensions.pocket.oAuthConsumerKey", // Use the opposite value as what default value the feed would have used - hidden: !PREFS_CONFIG.get("feeds.section.topstories").getValue(args), + hidden: !PREFS_CONFIG.get("feeds.system.topstories").getValue(args), provider_icon: "pocket", provider_name: "Pocket", read_more_endpoint: "https://getpocket.com/explore/trending?src=fx_new_tab", stories_endpoint: `https://getpocket.cdn.mozilla.net/v3/firefox/global-recs?version=3&consumer_key=$apiKey&locale_lang=${ args.locale }&feed_variant=${ showSpocs(args) ? "default_spocs_on" : "default_spocs_off" @@ -370,16 +370,24 @@ const PREFS_CONFIG = new Map([ [ "section.topstories.rows", { title: "Number of rows of Top Stories to display", value: 1, }, ], [ + "feeds.section.topstories", + { + title: + "User pref to show stories on newtab (feeds.system.topstories has to be set to true as well)", + value: true, + }, + ], + [ "sectionOrder", { title: "The rendering order for the sections", value: "topsites,topstories,highlights", }, ], [ "improvesearch.noDefaultSearchTile", @@ -596,21 +604,21 @@ const FEEDS_DATA = [ }, { name: "section.highlights", factory: () => new HighlightsFeed(), title: "Fetches content recommendations from places db", value: true, }, { - name: "section.topstories", + name: "system.topstories", factory: () => new TopStoriesFeed(PREFS_CONFIG.get("discoverystream.config")), title: - "Fetches content recommendations from a configurable content provider", + "System pref that fetches content recommendations from a configurable content provider", // Dynamically determine if Pocket should be shown for a geo / locale getValue: ({ geo, locale }) => { const preffedRegionsString = Services.prefs.getStringPref(REGION_STORIES_CONFIG) || ""; const preffedRegions = preffedRegionsString.split(",").map(s => s.trim()); const locales = { US: ["en-CA", "en-GB", "en-US"], CA: ["en-CA", "en-GB", "en-US"],
--- a/browser/components/newtab/test/unit/content-src/components/DiscoveryStreamBase.test.jsx +++ b/browser/components/newtab/test/unit/content-src/components/DiscoveryStreamBase.test.jsx @@ -102,16 +102,17 @@ describe("<DiscoveryStreamBase>", () => }; return shallow( <DiscoveryStreamBase locale="en-US" DiscoveryStream={defaultProps} Prefs={{ values: { "feeds.section.topstories": true, + "feeds.system.topstories": true, "feeds.topsites": true, }, }} document={{ documentElement: { lang: "en-US" }, }} Sections={[ {
--- a/browser/components/newtab/test/unit/content-src/lib/selectLayoutRender.test.js +++ b/browser/components/newtab/test/unit/content-src/lib/selectLayoutRender.test.js @@ -926,16 +926,17 @@ describe("selectLayoutRender", () => { type: at.DISCOVERY_STREAM_LAYOUT_UPDATE, data: { layout: fakeLayout }, }); const { layoutRender } = selectLayoutRender({ state: store.getState().DiscoveryStream, prefs: { "feeds.topsites": true, + "feeds.system.topstories": true, "feeds.section.topstories": true, }, }); assert.equal(layoutRender[0].components[0].type, "Message"); assert.equal(layoutRender[0].components[1].type, "TopSites"); assert.equal(layoutRender[0].components[2], undefined); });
--- a/browser/components/newtab/test/unit/lib/ActivityStream.test.js +++ b/browser/components/newtab/test/unit/lib/ActivityStream.test.js @@ -116,28 +116,23 @@ describe("ActivityStream", () => { it("should create a Telemetry feed", () => { const feed = as.feeds.get("feeds.telemetry")(); assert.instanceOf(feed, Fake); }); it("should create a Prefs feed", () => { const feed = as.feeds.get("feeds.prefs")(); assert.instanceOf(feed, Fake); }); - it("should create a section feed for each section in PREFS_CONFIG", () => { - // If new sections are added, their feeds will have to be added to the - // list of injected feeds above for this test to pass - let feedCount = 0; - for (const pref of PREFS_CONFIG.keys()) { - if (pref.search(/^feeds\.section\.[^.]+$/) === 0) { - const feed = as.feeds.get(pref)(); - assert.instanceOf(feed, Fake); - feedCount++; - } - } - assert.isAbove(feedCount, 0); + it("should create a HighlightsFeed feed", () => { + const feed = as.feeds.get("feeds.section.highlights")(); + assert.instanceOf(feed, Fake); + }); + it("should create a TopStoriesFeed feed", () => { + const feed = as.feeds.get("feeds.system.topstories")(); + assert.instanceOf(feed, Fake); }); it("should create a AboutPreferences feed", () => { const feed = as.feeds.get("feeds.aboutpreferences")(); assert.instanceOf(feed, Fake); }); it("should create a SectionsFeed", () => { const feed = as.feeds.get("feeds.sections")(); assert.instanceOf(feed, Fake); @@ -336,61 +331,61 @@ describe("ActivityStream", () => { }); it("should be false with no geo/locale", () => { prefHasUserValueStub.returns(false); appLocaleAsBCP47Stub.get(() => ""); getStringPrefStub.withArgs("browser.search.region").returns(""); as._updateDynamicPrefs(); - assert.isFalse(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should be false with unexpected geo", () => { getStringPrefStub.withArgs("browser.search.region").returns("NOGEO"); as._updateDynamicPrefs(); - assert.isFalse(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should be false with expected geo and unexpected locale", () => { appLocaleAsBCP47Stub.get(() => "no-LOCALE"); as._updateDynamicPrefs(); - assert.isFalse(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should be true with expected geo and locale", () => { as._updateDynamicPrefs(); - assert.isTrue(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isTrue(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should be false after expected geo and locale then unexpected", () => { getStringPrefStub .withArgs("browser.search.region") .onFirstCall() .returns("US") .onSecondCall() .returns("NOGEO"); as._updateDynamicPrefs(); as._updateDynamicPrefs(); - assert.isFalse(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should be true with updated pref change", () => { appLocaleAsBCP47Stub.get(() => "en-GB"); getStringPrefStub.withArgs("browser.search.region").returns("GB"); getStringPrefStub .withArgs( "browser.newtabpage.activity-stream.discoverystream.region-stories-config" ) .returns("US,CA,GB"); as._updateDynamicPrefs(); - assert.isTrue(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isTrue(PREFS_CONFIG.get("feeds.system.topstories").value); }); }); describe("_updateDynamicPrefs topstories delayed default value", () => { let clock; beforeEach(() => { clock = sinon.useFakeTimers(); // Have addObserver cause prefHasUserValue to now return true then observe @@ -404,40 +399,40 @@ describe("ActivityStream", () => { afterEach(() => clock.restore()); it("should set false with unexpected geo", () => { sandbox.stub(global.Services.prefs, "getStringPref").returns("NOGEO"); as._updateDynamicPrefs(); clock.tick(1); - assert.isFalse(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should set true with expected geo and locale", () => { sandbox.stub(global.Services.prefs, "getStringPref").returns("US"); sandbox .stub(global.Services.locale, "appLocaleAsBCP47") .get(() => "en-US"); as._updateDynamicPrefs(); clock.tick(1); - assert.isTrue(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isTrue(PREFS_CONFIG.get("feeds.system.topstories").value); }); it("should not change default even with expected geo and locale", () => { - as._defaultPrefs.set("feeds.section.topstories", false); + as._defaultPrefs.set("feeds.system.topstories", false); sandbox.stub(global.Services.prefs, "getStringPref").returns("US"); sandbox .stub(global.Services.locale, "appLocaleAsBCP47") .get(() => "en-US"); as._updateDynamicPrefs(); clock.tick(1); - assert.isFalse(PREFS_CONFIG.get("feeds.section.topstories").value); + assert.isFalse(PREFS_CONFIG.get("feeds.system.topstories").value); }); }); describe("telemetry reporting on init failure", () => { it("should send a ping on init error", () => { as = new ActivityStream(); const telemetry = { handleUndesiredEvent: sandbox.spy() }; sandbox.stub(as.store, "init").throws(); sandbox.stub(as.store.feeds, "get").returns(telemetry);