From 4cec5cd2f1d8936656eadf05423e61e7cbccb4e9 Mon Sep 17 00:00:00 2001 From: Andrei Oprea Date: Thu, 15 Nov 2018 08:54:00 +0000 Subject: [PATCH] Fix Bug 1506594 - Open snippet links in same window (#4563) --- content-src/asrouter/components/RichText/RichText.jsx | 6 +++--- .../templates/SubmitFormSnippet/SubmitFormSnippet.jsx | 2 ++ lib/ASRouter.jsm | 4 ++-- lib/OnboardingMessageProvider.jsm | 6 +++--- test/unit/asrouter/ASRouter.test.js | 4 ++-- test/unit/asrouter/RichText.test.jsx | 8 ++++++++ 6 files changed, 20 insertions(+), 10 deletions(-) diff --git a/content-src/asrouter/components/RichText/RichText.jsx b/content-src/asrouter/components/RichText/RichText.jsx index bf462c29a..f9eab80db 100644 --- a/content-src/asrouter/components/RichText/RichText.jsx +++ b/content-src/asrouter/components/RichText/RichText.jsx @@ -17,7 +17,7 @@ const ALLOWED_TAGS = { * Transform an object (tag name: {url}) into (tag name: anchor) where the url * is used as href, in order to render links inside a Fluent.Localized component. */ -export function convertLinks(links, sendClick, doNotAutoBlock) { +export function convertLinks(links, sendClick, doNotAutoBlock, openNewWindow = false) { if (links) { return Object.keys(links).reduce((acc, linkTag) => { const {action} = links[linkTag]; @@ -25,7 +25,7 @@ export function convertLinks(links, sendClick, doNotAutoBlock) { const url = action ? false : safeURI(links[linkTag].url); acc[linkTag] = ( + {props.text} ); diff --git a/content-src/asrouter/templates/SubmitFormSnippet/SubmitFormSnippet.jsx b/content-src/asrouter/templates/SubmitFormSnippet/SubmitFormSnippet.jsx index 61575d56c..727aa39a6 100644 --- a/content-src/asrouter/templates/SubmitFormSnippet/SubmitFormSnippet.jsx +++ b/content-src/asrouter/templates/SubmitFormSnippet/SubmitFormSnippet.jsx @@ -89,6 +89,7 @@ export class SubmitFormSnippet extends React.PureComponent { localization_id="disclaimer_html" links={content.links} doNotAutoBlock={true} + openNewWindow={true} sendClick={this.props.sendClick} />

); } @@ -105,6 +106,7 @@ export class SubmitFormSnippet extends React.PureComponent { localization_id="privacy_html" links={content.links} doNotAutoBlock={true} + openNewWindow={true} sendClick={this.props.sendClick} />

diff --git a/lib/ASRouter.jsm b/lib/ASRouter.jsm index 059faf048..6e530345e 100644 --- a/lib/ASRouter.jsm +++ b/lib/ASRouter.jsm @@ -928,7 +928,7 @@ class _ASRouter { target.browser.ownerGlobal.OpenBrowserWindow({private: true}); break; case ra.OPEN_URL: - target.browser.ownerGlobal.openLinkIn(action.data.args, "tabshifted", { + target.browser.ownerGlobal.openLinkIn(action.data.args, action.data.where || "current", { private: false, triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal({}), }); @@ -948,7 +948,7 @@ class _ASRouter { case ra.SHOW_FIREFOX_ACCOUNTS: const url = await FxAccounts.config.promiseSignUpURI("snippets"); // We want to replace the current tab. - target.browser.ownerGlobal.openLinkIn(url, "tabshifted", { + target.browser.ownerGlobal.openLinkIn(url, "current", { private: false, triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal({}), }); diff --git a/lib/OnboardingMessageProvider.jsm b/lib/OnboardingMessageProvider.jsm index 68df56d40..f118b30f3 100644 --- a/lib/OnboardingMessageProvider.jsm +++ b/lib/OnboardingMessageProvider.jsm @@ -38,7 +38,7 @@ const ONBOARDING_MESSAGES = async () => ([ button_label: {string_id: "onboarding-button-label-try-now"}, button_action: { type: "OPEN_URL", - data: {args: "https://screenshots.firefox.com/#tour"}, + data: {args: "https://screenshots.firefox.com/#tour", where: "tabshifted"}, }, }, trigger: {id: "firstRun"}, @@ -73,7 +73,7 @@ const ONBOARDING_MESSAGES = async () => ([ button_label: {string_id: "onboarding-button-label-try-now"}, button_action: { type: "OPEN_URL", - data: {args: "https://addons.mozilla.org/en-US/firefox/addon/ghostery/"}, + data: {args: "https://addons.mozilla.org/en-US/firefox/addon/ghostery/", where: "tabshifted"}, }, }, targeting: "providerCohorts.onboarding == 'ghostery'", @@ -91,7 +91,7 @@ const ONBOARDING_MESSAGES = async () => ([ button_label: {string_id: "onboarding-button-label-get-started"}, button_action: { type: "OPEN_URL", - data: {args: await FxAccountsConfig.promiseEmailFirstURI("onboarding")}, + data: {args: await FxAccountsConfig.promiseEmailFirstURI("onboarding"), where: "tabshifted"}, }, }, targeting: "attributionData.campaign == 'non-fx-button' && attributionData.source == 'addons.mozilla.org'", diff --git a/test/unit/asrouter/ASRouter.test.js b/test/unit/asrouter/ASRouter.test.js index c134a1c16..47b9b428f 100644 --- a/test/unit/asrouter/ASRouter.test.js +++ b/test/unit/asrouter/ASRouter.test.js @@ -840,7 +840,7 @@ describe("ASRouter", () => { }); it("should call openLinkIn with the correct params on OPEN_URL", async () => { let [testMessage] = Router.state.messages; - testMessage.button_action = {type: "OPEN_URL", data: {args: "some/url.com"}}; + testMessage.button_action = {type: "OPEN_URL", data: {args: "some/url.com", where: "tabshifted"}}; const msg = fakeExecuteUserAction(testMessage.button_action); await Router.onMessage(msg); @@ -873,7 +873,7 @@ describe("ASRouter", () => { assert.calledOnce(msg.target.browser.ownerGlobal.openLinkIn); assert.calledWith(msg.target.browser.ownerGlobal.openLinkIn, - "some/url", "tabshifted", {"private": false, "triggeringPrincipal": undefined}); + "some/url", "current", {"private": false, "triggeringPrincipal": undefined}); }); }); diff --git a/test/unit/asrouter/RichText.test.jsx b/test/unit/asrouter/RichText.test.jsx index 6e27fd043..9b1420217 100644 --- a/test/unit/asrouter/RichText.test.jsx +++ b/test/unit/asrouter/RichText.test.jsx @@ -43,6 +43,14 @@ describe("convertLinks", () => { assert.propertyVal(result.cta.props, "data-args", cta.args); assert.propertyVal(result.cta.props, "onClick", stub); }); + it("should follow openNewWindow prop", () => { + const cta = {url: "https://foo.com"}; + const newWindow = convertLinks({cta}, sandbox.stub(), false, true); + const sameWindow = convertLinks({cta}, sandbox.stub(), false); + + assert.propertyVal(newWindow.cta.props, "target", "_blank"); + assert.propertyVal(sameWindow.cta.props, "target", ""); + }); it("should allow for custom elements & styles", () => { const wrapper = mount(}} text="foo" localization_id="text" />);