From 8bd4b27ad8e0f5c47f152490349e4b3177e777fb Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Mon, 24 Jul 2017 18:18:51 -0400 Subject: [PATCH] feat(systemaddon): Add HTTP Referrer to Top Stories. Closes #2940. --- content-test/components/LinkMenu.test.js | 4 +- system-addon/common/Actions.jsm | 1 + .../content-src/components/Card/Card.jsx | 8 +++- .../content-src/lib/link-menu-options.js | 4 +- system-addon/lib/ActivityStream.jsm | 1 + system-addon/lib/PlacesFeed.jsm | 22 ++++++++++ system-addon/lib/TopSitesFeed.jsm | 10 ----- system-addon/lib/TopStoriesFeed.jsm | 2 + .../content-src/components/LinkMenu.test.jsx | 6 +-- system-addon/test/unit/lib/PlacesFeed.test.js | 43 +++++++++++++++++++ .../test/unit/lib/TopSitesFeed.test.js | 22 ---------- .../test/unit/lib/TopStoriesFeed.test.js | 4 ++ 12 files changed, 87 insertions(+), 40 deletions(-) diff --git a/content-test/components/LinkMenu.test.js b/content-test/components/LinkMenu.test.js index 898e28ceca..b43cbf05cc 100644 --- a/content-test/components/LinkMenu.test.js +++ b/content-test/components/LinkMenu.test.js @@ -137,13 +137,13 @@ describe("LinkMenu", () => { checkOption({ ref: "openWindow", event: "NOTIFY_OPEN_WINDOW", - eventData: {url: DEFAULT_PROPS.site.url}, + eventData: {url: DEFAULT_PROPS.site.url, referrer: "ref"}, userEvent: "OPEN_NEW_WINDOW" }); checkOption({ ref: "openPrivate", event: "NOTIFY_OPEN_WINDOW", - eventData: {url: DEFAULT_PROPS.site.url, isPrivate: true}, + eventData: {url: DEFAULT_PROPS.site.url, referrer: "ref", isPrivate: true}, userEvent: "OPEN_PRIVATE_WINDOW" }); checkOption({ diff --git a/system-addon/common/Actions.jsm b/system-addon/common/Actions.jsm index 178ec06192..6c5212ca4d 100644 --- a/system-addon/common/Actions.jsm +++ b/system-addon/common/Actions.jsm @@ -40,6 +40,7 @@ for (const type of [ "NEW_TAB_LOAD", "NEW_TAB_UNLOAD", "NEW_TAB_VISIBLE", + "OPEN_LINK", "OPEN_NEW_WINDOW", "OPEN_PRIVATE_WINDOW", "PINNED_SITES_UPDATED", diff --git a/system-addon/content-src/components/Card/Card.jsx b/system-addon/content-src/components/Card/Card.jsx index a324a6d491..fefe6dae0b 100644 --- a/system-addon/content-src/components/Card/Card.jsx +++ b/system-addon/content-src/components/Card/Card.jsx @@ -3,6 +3,7 @@ const LinkMenu = require("content-src/components/LinkMenu/LinkMenu"); const shortURL = require("content-src/lib/short-url"); const {FormattedMessage} = require("react-intl"); const cardContextTypes = require("./types"); +const {actionCreators: ac, actionTypes: at} = require("common/Actions.jsm"); /** * Card component. @@ -19,6 +20,7 @@ class Card extends React.Component { this.state = {showContextMenu: false, activeCard: null}; this.onMenuButtonClick = this.onMenuButtonClick.bind(this); this.onMenuUpdate = this.onMenuUpdate.bind(this); + this.onClick = this.onClick.bind(this); } onMenuButtonClick(event) { event.preventDefault(); @@ -27,6 +29,10 @@ class Card extends React.Component { showContextMenu: true }); } + onClick(event) { + event.preventDefault(); + this.props.dispatch(ac.SendToMain({type: at.OPEN_LINK, data: this.props.link})); + } onMenuUpdate(showContextMenu) { this.setState({showContextMenu}); } @@ -37,7 +43,7 @@ class Card extends React.Component { const {icon, intlID} = cardContextTypes[link.type]; return (
  • - +
    {link.image &&
    }
    diff --git a/system-addon/content-src/lib/link-menu-options.js b/system-addon/content-src/lib/link-menu-options.js index 92d6b7dcfb..a185084173 100644 --- a/system-addon/content-src/lib/link-menu-options.js +++ b/system-addon/content-src/lib/link-menu-options.js @@ -31,7 +31,7 @@ module.exports = { icon: "new-window", action: ac.SendToMain({ type: at.OPEN_NEW_WINDOW, - data: {url: site.url} + data: {url: site.url, referrer: site.referrer} }), userEvent: "OPEN_NEW_WINDOW" }), @@ -40,7 +40,7 @@ module.exports = { icon: "new-window-private", action: ac.SendToMain({ type: at.OPEN_PRIVATE_WINDOW, - data: {url: site.url} + data: {url: site.url, referrer: site.referrer} }), userEvent: "OPEN_PRIVATE_WINDOW" }), diff --git a/system-addon/lib/ActivityStream.jsm b/system-addon/lib/ActivityStream.jsm index 2fd01c1ba4..c2e23eac78 100644 --- a/system-addon/lib/ActivityStream.jsm +++ b/system-addon/lib/ActivityStream.jsm @@ -74,6 +74,7 @@ const PREFS_CONFIG = new Map([ title: "Configuration options for top stories feed", value: `{ "stories_endpoint": "https://getpocket.com/v3/firefox/global-recs?consumer_key=$apiKey", + "stories_referrer": "https://getpocket.com/recommendations", "topics_endpoint": "https://getpocket.com/v3/firefox/trending-topics?consumer_key=$apiKey", "read_more_endpoint": "https://getpocket.com/explore/trending?src=ff_new_tab", "learn_more_endpoint": "https://getpocket.com/firefox_learnmore?src=ff_newtab", diff --git a/system-addon/lib/PlacesFeed.jsm b/system-addon/lib/PlacesFeed.jsm index 3de9568b47..f919427ee7 100644 --- a/system-addon/lib/PlacesFeed.jsm +++ b/system-addon/lib/PlacesFeed.jsm @@ -186,6 +186,14 @@ class PlacesFeed { } } + openNewWindow(action, isPrivate = false) { + const win = action._target.browser.ownerGlobal; + const privateParam = {private: isPrivate}; + const params = (action.data.referrer) ? + Object.assign(privateParam, {referrerURI: Services.io.newURI(action.data.referrer)}) : privateParam; + win.openLinkIn(action.data.url, "window", params); + } + onAction(action) { switch (action.type) { case at.INIT: @@ -207,9 +215,23 @@ class PlacesFeed { case at.DELETE_HISTORY_URL: NewTabUtils.activityStreamLinks.deleteHistoryEntry(action.data); break; + case at.OPEN_NEW_WINDOW: + this.openNewWindow(action); + break; + case at.OPEN_PRIVATE_WINDOW: + this.openNewWindow(action, true); + break; case at.SAVE_TO_POCKET: Pocket.savePage(action._target.browser, action.data.site.url, action.data.site.title); break; + case at.OPEN_LINK: { + if (action.data.referrer) { + action._target.browser.loadURI(action.data.url, Services.io.newURI(action.data.referrer)); + } else { + action._target.browser.loadURI(action.data.url); + } + break; + } } } } diff --git a/system-addon/lib/TopSitesFeed.jsm b/system-addon/lib/TopSitesFeed.jsm index 7e70b69838..b2cb207879 100644 --- a/system-addon/lib/TopSitesFeed.jsm +++ b/system-addon/lib/TopSitesFeed.jsm @@ -88,10 +88,6 @@ this.TopSitesFeed = class TopSitesFeed { } this.lastUpdated = Date.now(); } - openNewWindow(action, isPrivate = false) { - const win = action._target.browser.ownerGlobal; - win.openLinkIn(action.data.url, "window", {private: isPrivate}); - } _getPinnedWithData() { // Augment the pinned links with any other extra data we have for them already in the store const links = this.store.getState().TopSites.rows; @@ -134,12 +130,6 @@ this.TopSitesFeed = class TopSitesFeed { this.refresh(action.meta.fromTarget); } break; - case at.OPEN_NEW_WINDOW: - this.openNewWindow(action); - break; - case at.OPEN_PRIVATE_WINDOW: - this.openNewWindow(action, true); - break; case at.PLACES_HISTORY_CLEARED: this.refresh(); break; diff --git a/system-addon/lib/TopStoriesFeed.jsm b/system-addon/lib/TopStoriesFeed.jsm index 6945350ad4..4e8be91896 100644 --- a/system-addon/lib/TopStoriesFeed.jsm +++ b/system-addon/lib/TopStoriesFeed.jsm @@ -31,6 +31,7 @@ this.TopStoriesFeed = class TopStoriesFeed { this.stories_endpoint = this._produceUrlWithApiKey(options.stories_endpoint, apiKey); this.topics_endpoint = this._produceUrlWithApiKey(options.topics_endpoint, apiKey); this.read_more_endpoint = options.read_more_endpoint; + this.stories_referrer = options.stories_referrer; // TODO https://github.com/mozilla/activity-stream/issues/2902 const sectionOptions = { @@ -85,6 +86,7 @@ this.TopStoriesFeed = class TopStoriesFeed { "title": s.title, "description": s.excerpt, "image": this._normalizeUrl(s.image_src), + "referrer": this.stories_referrer, "url": s.dedupe_url })); return items; diff --git a/system-addon/test/unit/content-src/components/LinkMenu.test.jsx b/system-addon/test/unit/content-src/components/LinkMenu.test.jsx index b0bdba263c..6b21616162 100644 --- a/system-addon/test/unit/content-src/components/LinkMenu.test.jsx +++ b/system-addon/test/unit/content-src/components/LinkMenu.test.jsx @@ -91,14 +91,14 @@ describe("", () => { describe(".onClick", () => { const FAKE_INDEX = 3; const FAKE_SOURCE = "TOP_SITES"; - const FAKE_SITE = {url: "https://foo.com", title: "bar", bookmarkGuid: 1234}; + const FAKE_SITE = {url: "https://foo.com", referrer: "https://foo.com/ref", title: "bar", bookmarkGuid: 1234}; const dispatch = sinon.stub(); const propOptions = ["Separator", "RemoveBookmark", "AddBookmark", "OpenInNewWindow", "OpenInPrivateWindow", "BlockUrl", "DeleteUrl", "PinTopSite", "UnpinTopSite", "SaveToPocket"]; const expectedActionData = { menu_action_remove_bookmark: FAKE_SITE.bookmarkGuid, menu_action_bookmark: FAKE_SITE.url, - menu_action_open_new_window: {url: FAKE_SITE.url}, - menu_action_open_private_window: {url: FAKE_SITE.url}, + menu_action_open_new_window: {url: FAKE_SITE.url, referrer: FAKE_SITE.referrer}, + menu_action_open_private_window: {url: FAKE_SITE.url, referrer: FAKE_SITE.referrer}, menu_action_dismiss: FAKE_SITE.url, menu_action_delete: FAKE_SITE.url, menu_action_pin: {site: {url: FAKE_SITE.url, title: shortURL(FAKE_SITE)}, index: FAKE_INDEX}, diff --git a/system-addon/test/unit/lib/PlacesFeed.test.js b/system-addon/test/unit/lib/PlacesFeed.test.js index 56bdcdc5d5..b3e5fec2b4 100644 --- a/system-addon/test/unit/lib/PlacesFeed.test.js +++ b/system-addon/test/unit/lib/PlacesFeed.test.js @@ -99,6 +99,49 @@ describe("PlacesFeed", () => { feed.onAction({type: at.DELETE_HISTORY_URL, data: "guava.com"}); assert.calledWith(global.NewTabUtils.activityStreamLinks.deleteHistoryEntry, "guava.com"); }); + it("should call openNewWindow with the correct url on OPEN_NEW_WINDOW", () => { + sinon.stub(feed, "openNewWindow"); + const openWindowAction = {type: at.OPEN_NEW_WINDOW, data: {url: "foo.com"}}; + feed.onAction(openWindowAction); + assert.calledWith(feed.openNewWindow, openWindowAction); + }); + it("should call openNewWindow with the correct url and privacy args on OPEN_PRIVATE_WINDOW", () => { + sinon.stub(feed, "openNewWindow"); + const openWindowAction = {type: at.OPEN_PRIVATE_WINDOW, data: {url: "foo.com"}}; + feed.onAction(openWindowAction); + assert.calledWith(feed.openNewWindow, openWindowAction, true); + }); + it("should call openNewWindow with the correct url on OPEN_NEW_WINDOW", () => { + const openWindowAction = { + type: at.OPEN_NEW_WINDOW, + data: {url: "foo.com"}, + _target: {browser: {ownerGlobal: {openLinkIn: () => {}}}} + }; + sinon.stub(openWindowAction._target.browser.ownerGlobal, "openLinkIn"); + feed.onAction(openWindowAction); + assert.calledOnce(openWindowAction._target.browser.ownerGlobal.openLinkIn); + }); + it("should open link on OPEN_LINK", () => { + sinon.stub(feed, "openNewWindow"); + const openLinkAction = { + type: at.OPEN_LINK, + data: {url: "foo.com"}, + _target: {browser: {loadURI: sinon.spy()}} + }; + feed.onAction(openLinkAction); + assert.calledWith(openLinkAction._target.browser.loadURI, openLinkAction.data.url); + }); + it("should open link with referrer on OPEN_LINK", () => { + globals.set("Services", {io: {newURI: url => `URI:${url}`}}); + sinon.stub(feed, "openNewWindow"); + const openLinkAction = { + type: at.OPEN_LINK, + data: {url: "foo.com", referrer: "foo.com/ref"}, + _target: {browser: {loadURI: sinon.spy()}} + }; + feed.onAction(openLinkAction); + assert.calledWith(openLinkAction._target.browser.loadURI, openLinkAction.data.url, `URI:${openLinkAction.data.referrer}`); + }); it("should save to Pocket on SAVE_TO_POCKET", () => { feed.onAction({type: at.SAVE_TO_POCKET, data: {site: {url: "raspberry.com", title: "raspberry"}}, _target: {browser: {}}}); assert.calledWith(global.Pocket.savePage, {}, "raspberry.com", "raspberry"); diff --git a/system-addon/test/unit/lib/TopSitesFeed.test.js b/system-addon/test/unit/lib/TopSitesFeed.test.js index a37d589175..9c24b48e1d 100644 --- a/system-addon/test/unit/lib/TopSitesFeed.test.js +++ b/system-addon/test/unit/lib/TopSitesFeed.test.js @@ -165,28 +165,6 @@ describe("Top Sites Feed", () => { feed.onAction(newTabAction); assert.notCalled(feed.refresh); }); - it("should call openNewWindow with the correct url on OPEN_NEW_WINDOW", () => { - sinon.stub(feed, "openNewWindow"); - const openWindowAction = {type: at.OPEN_NEW_WINDOW, data: {url: "foo.com"}}; - feed.onAction(openWindowAction); - assert.calledWith(feed.openNewWindow, openWindowAction); - }); - it("should call openNewWindow with the correct url and privacy args on OPEN_PRIVATE_WINDOW", () => { - sinon.stub(feed, "openNewWindow"); - const openWindowAction = {type: at.OPEN_PRIVATE_WINDOW, data: {url: "foo.com"}}; - feed.onAction(openWindowAction); - assert.calledWith(feed.openNewWindow, openWindowAction, true); - }); - it("should call openNewWindow with the correct url on OPEN_NEW_WINDOW", () => { - const openWindowAction = { - type: at.OPEN_NEW_WINDOW, - data: {url: "foo.com"}, - _target: {browser: {ownerGlobal: {openLinkIn: () => {}}}} - }; - sinon.stub(openWindowAction._target.browser.ownerGlobal, "openLinkIn"); - feed.onAction(openWindowAction); - assert.calledOnce(openWindowAction._target.browser.ownerGlobal.openLinkIn); - }); it("should call with correct parameters on TOP_SITES_PIN", () => { const pinAction = { type: at.TOP_SITES_PIN, diff --git a/system-addon/test/unit/lib/TopStoriesFeed.test.js b/system-addon/test/unit/lib/TopStoriesFeed.test.js index 4443dac0fe..dd799b15f8 100644 --- a/system-addon/test/unit/lib/TopStoriesFeed.test.js +++ b/system-addon/test/unit/lib/TopStoriesFeed.test.js @@ -16,6 +16,7 @@ describe("Top Stories Feed", () => { beforeEach(() => { FakePrefs.prototype.prefs["feeds.section.topstories.options"] = `{ "stories_endpoint": "https://somedomain.org/stories?key=$apiKey", + "stories_referrer": "https://somedomain.org/referrer", "topics_endpoint": "https://somedomain.org/topics?key=$apiKey", "survey_link": "https://www.surveymonkey.com/r/newtabffx", "api_key_pref": "apiKeyPref", @@ -42,6 +43,7 @@ describe("Top Stories Feed", () => { it("should initialize endpoints based on prefs", () => { instance.onAction({type: at.INIT}); assert.equal("https://somedomain.org/stories?key=test-api-key", instance.stories_endpoint); + assert.equal("https://somedomain.org/referrer", instance.stories_referrer); assert.equal("https://somedomain.org/topics?key=test-api-key", instance.topics_endpoint); }); it("should register section", () => { @@ -145,10 +147,12 @@ describe("Top Stories Feed", () => { "title": "title", "description": "description", "image": "image-url", + "referrer": "referrer", "url": "rec-url" }]; instance.stories_endpoint = "stories-endpoint"; + instance.stories_referrer = "referrer"; fetchStub.resolves({ok: true, status: 200, text: () => response}); await instance.fetchStories();