From 5f5be9298849652f0b5c339c9bc89b64d7faf0fd Mon Sep 17 00:00:00 2001 From: Andrei Oprea Date: Thu, 3 Aug 2017 14:44:30 +0200 Subject: [PATCH] fix(systemaddon): Limit the number of topsites being rendered. Closes #2945. --- system-addon/common/Reducers.jsm | 6 ++++-- system-addon/lib/TopSitesFeed.jsm | 6 ++---- system-addon/test/unit/common/Reducers.test.js | 12 +++++++++++- system-addon/test/unit/lib/TopSitesFeed.test.js | 10 +++++----- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/system-addon/common/Reducers.jsm b/system-addon/common/Reducers.jsm index eb37464cdf..6c6e70cc6b 100644 --- a/system-addon/common/Reducers.jsm +++ b/system-addon/common/Reducers.jsm @@ -5,6 +5,8 @@ const {actionTypes: at} = Components.utils.import("resource://activity-stream/common/Actions.jsm", {}); +const TOP_SITES_SHOWMORE_LENGTH = 12; + const INITIAL_STATE = { App: { // Have we received real data from the app yet? @@ -139,7 +141,7 @@ function TopSites(prevState = INITIAL_STATE.TopSites, action) { return Object.assign({}, prevState, {rows: newRows}); case at.PINNED_SITES_UPDATED: pinned = action.data; - newRows = insertPinned(prevState.rows, pinned); + newRows = insertPinned(prevState.rows, pinned).slice(0, TOP_SITES_SHOWMORE_LENGTH); return Object.assign({}, prevState, {rows: newRows}); default: return prevState; @@ -258,4 +260,4 @@ this.INITIAL_STATE = INITIAL_STATE; this.reducers = {TopSites, App, Snippets, Prefs, Dialog, Sections}; this.insertPinned = insertPinned; -this.EXPORTED_SYMBOLS = ["reducers", "INITIAL_STATE", "insertPinned"]; +this.EXPORTED_SYMBOLS = ["reducers", "INITIAL_STATE", "insertPinned", "TOP_SITES_SHOWMORE_LENGTH"]; diff --git a/system-addon/lib/TopSitesFeed.jsm b/system-addon/lib/TopSitesFeed.jsm index 9069269842..7dd8e40203 100644 --- a/system-addon/lib/TopSitesFeed.jsm +++ b/system-addon/lib/TopSitesFeed.jsm @@ -8,7 +8,7 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm"); const {actionCreators: ac, actionTypes: at} = Cu.import("resource://activity-stream/common/Actions.jsm", {}); const {TippyTopProvider} = Cu.import("resource://activity-stream/lib/TippyTopProvider.jsm", {}); -const {insertPinned} = Cu.import("resource://activity-stream/common/Reducers.jsm", {}); +const {insertPinned, TOP_SITES_SHOWMORE_LENGTH} = Cu.import("resource://activity-stream/common/Reducers.jsm", {}); const {Dedupe} = Cu.import("resource://activity-stream/common/Dedupe.jsm", {}); const {shortURL} = Cu.import("resource://activity-stream/common/ShortURL.jsm", {}); @@ -17,7 +17,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "NewTabUtils", XPCOMUtils.defineLazyModuleGetter(this, "Screenshots", "resource://activity-stream/lib/Screenshots.jsm"); -const TOP_SITES_SHOWMORE_LENGTH = 12; const UPDATE_TIME = 15 * 60 * 1000; // 15 minutes const DEFAULT_SITES_PREF = "default.sites"; const DEFAULT_TOP_SITES = []; @@ -170,6 +169,5 @@ this.TopSitesFeed = class TopSitesFeed { }; this.UPDATE_TIME = UPDATE_TIME; -this.TOP_SITES_SHOWMORE_LENGTH = TOP_SITES_SHOWMORE_LENGTH; this.DEFAULT_TOP_SITES = DEFAULT_TOP_SITES; -this.EXPORTED_SYMBOLS = ["TopSitesFeed", "UPDATE_TIME", "DEFAULT_TOP_SITES", "TOP_SITES_SHOWMORE_LENGTH"]; +this.EXPORTED_SYMBOLS = ["TopSitesFeed", "UPDATE_TIME", "DEFAULT_TOP_SITES"]; diff --git a/system-addon/test/unit/common/Reducers.test.js b/system-addon/test/unit/common/Reducers.test.js index aa4011926f..48757e92af 100644 --- a/system-addon/test/unit/common/Reducers.test.js +++ b/system-addon/test/unit/common/Reducers.test.js @@ -1,4 +1,4 @@ -const {reducers, INITIAL_STATE, insertPinned} = require("common/Reducers.jsm"); +const {reducers, INITIAL_STATE, insertPinned, TOP_SITES_SHOWMORE_LENGTH} = require("common/Reducers.jsm"); const {TopSites, App, Snippets, Prefs, Dialog, Sections} = reducers; const {actionTypes: at} = require("common/Actions.jsm"); @@ -126,6 +126,16 @@ describe("Reducers", () => { const nextState = TopSites(oldState, action); assert.deepEqual(nextState.rows, [{url: "baz.com", title: "baz", isPinned: true, pinIndex: 0, pinTitle: "baz"}, {url: "foo.com"}, {url: "bar.com"}]); }); + it("should return at most TOP_SITES_SHOWMORE_LENGTH sites on PINNED_SITES_UPDATED", () => { + const oldState = {rows: [{url: "foo.com"}, {url: "bar.com"}]}; + const data = new Array(20).fill(null).map((s, i) => ({ + url: "foo.com", + pinIndex: i + })); + const action = {type: at.PINNED_SITES_UPDATED, data}; + const nextState = TopSites(oldState, action); + assert.lengthOf(nextState.rows, TOP_SITES_SHOWMORE_LENGTH); + }); }); describe("Prefs", () => { function prevState(custom = {}) { diff --git a/system-addon/test/unit/lib/TopSitesFeed.test.js b/system-addon/test/unit/lib/TopSitesFeed.test.js index 8963e99283..6e86f1bc3e 100644 --- a/system-addon/test/unit/lib/TopSitesFeed.test.js +++ b/system-addon/test/unit/lib/TopSitesFeed.test.js @@ -1,10 +1,10 @@ "use strict"; const injector = require("inject!lib/TopSitesFeed.jsm"); -const {UPDATE_TIME, TOP_SITES_SHOWMORE_LENGTH} = require("lib/TopSitesFeed.jsm"); +const {UPDATE_TIME} = require("lib/TopSitesFeed.jsm"); const {FakePrefs, GlobalOverrider} = require("test/unit/utils"); const action = {meta: {fromTarget: {}}}; const {actionCreators: ac, actionTypes: at} = require("common/Actions.jsm"); -const {insertPinned} = require("common/Reducers.jsm"); +const {insertPinned, TOP_SITES_SHOWMORE_LENGTH} = require("common/Reducers.jsm"); const FAKE_LINKS = new Array(TOP_SITES_SHOWMORE_LENGTH).fill(null).map((v, i) => ({url: `http://www.site${i}.com`})); const FAKE_SCREENSHOT = "data123"; @@ -46,7 +46,7 @@ describe("Top Sites Feed", () => { ({TopSitesFeed, DEFAULT_TOP_SITES} = injector({ "lib/ActivityStreamPrefs.jsm": {Prefs: FakePrefs}, "common/Dedupe.jsm": {Dedupe: fakeDedupe}, - "common/Reducers.jsm": {insertPinned}, + "common/Reducers.jsm": {insertPinned, TOP_SITES_SHOWMORE_LENGTH}, "lib/Screenshots.jsm": {Screenshots: fakeScreenshot}, "lib/TippyTopProvider.jsm": {TippyTopProvider: FakeTippyTopProvider}, "common/ShortURL.jsm": {shortURL: shortURLStub} @@ -144,7 +144,7 @@ describe("Top Sites Feed", () => { beforeEach(() => { ({TopSitesFeed, DEFAULT_TOP_SITES} = injector({ "lib/ActivityStreamPrefs.jsm": {Prefs: FakePrefs}, - "common/Reducers.jsm": {insertPinned}, + "common/Reducers.jsm": {insertPinned, TOP_SITES_SHOWMORE_LENGTH}, "lib/Screenshots.jsm": {Screenshots: fakeScreenshot} })); feed = new TopSitesFeed(); @@ -157,7 +157,7 @@ describe("Top Sites Feed", () => { const sites = await feed.getLinksWithDefaults(); - assert.lengthOf(sites, 12); + assert.lengthOf(sites, TOP_SITES_SHOWMORE_LENGTH); assert.equal(sites[0].url, fakeNewTabUtils.pinnedLinks.links[0].url); assert.equal(sites[1].url, fakeNewTabUtils.pinnedLinks.links[1].url); assert.equal(sites[0].hostname, sites[1].hostname);