From ceddaceffd3e0700e01cc632fe9a841fcb680433 Mon Sep 17 00:00:00 2001 From: Punam Dahiya Date: Tue, 13 Oct 2020 22:25:48 +0000 Subject: [PATCH] Bug 1670785 - Fix ASRouter devtools force attribution on MacOS r=andreio Differential Revision: https://phabricator.services.mozilla.com/D93299 --- .../components/ASRouterAdmin/ASRouterAdmin.jsx | 3 ++- .../data/content/activity-stream.bundle.js | 2 +- browser/components/newtab/lib/ASRouter.jsm | 18 ++++++++---------- .../newtab/test/unit/asrouter/ASRouter.test.js | 8 ++++++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx b/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx index 4229f2c419b5..4e6953c116e2 100644 --- a/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx +++ b/browser/components/newtab/content-src/components/ASRouterAdmin/ASRouterAdmin.jsx @@ -1461,7 +1461,8 @@ export class ASRouterAdminInner extends React.PureComponent { different attribution parameters, enter them in the text boxes. If you wish to try a different addon with the Return To AMO flow, make sure the 'content' text box has the addon GUID, then click 'Force - Attribution'. + Attribution'. Clicking on 'Force Attribution' with blank text boxes + reset attribution data.

diff --git a/browser/components/newtab/data/content/activity-stream.bundle.js b/browser/components/newtab/data/content/activity-stream.bundle.js index 8de9822c5d8a..7b9e6df63aac 100644 --- a/browser/components/newtab/data/content/activity-stream.bundle.js +++ b/browser/components/newtab/data/content/activity-stream.bundle.js @@ -1915,7 +1915,7 @@ class ASRouterAdminInner extends react__WEBPACK_IMPORTED_MODULE_4___default.a.Pu } renderAttributionParamers() { - return react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("div", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("h2", null, " Attribution Parameters "), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("p", null, " ", "This forces the browser to set some attribution parameters, useful for testing the Return To AMO feature. Clicking on 'Force Attribution', with the default values in each field, will demo the Return To AMO flow with the addon called 'Iridium for Youtube'. If you wish to try different attribution parameters, enter them in the text boxes. If you wish to try a different addon with the Return To AMO flow, make sure the 'content' text box has the addon GUID, then click 'Force Attribution'."), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("table", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("tr", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("b", null, " Source ")), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, " ", react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("input", { + return react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("div", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("h2", null, " Attribution Parameters "), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("p", null, " ", "This forces the browser to set some attribution parameters, useful for testing the Return To AMO feature. Clicking on 'Force Attribution', with the default values in each field, will demo the Return To AMO flow with the addon called 'Iridium for Youtube'. If you wish to try different attribution parameters, enter them in the text boxes. If you wish to try a different addon with the Return To AMO flow, make sure the 'content' text box has the addon GUID, then click 'Force Attribution'. Clicking on 'Force Attribution' with blank text boxes reset attribution data."), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("table", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("tr", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("b", null, " Source ")), react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("td", null, " ", react__WEBPACK_IMPORTED_MODULE_4___default.a.createElement("input", { type: "text", name: "source", placeholder: "addons.mozilla.org", diff --git a/browser/components/newtab/lib/ASRouter.jsm b/browser/components/newtab/lib/ASRouter.jsm index 788ae944096e..448ed24d7839 100644 --- a/browser/components/newtab/lib/ASRouter.jsm +++ b/browser/components/newtab/lib/ASRouter.jsm @@ -32,6 +32,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { SpecialMessageActions: "resource://messaging-system/lib/SpecialMessageActions.jsm", TargetingContext: "resource://messaging-system/targeting/Targeting.jsm", + MacAttribution: "resource:///modules/MacAttribution.jsm", }); XPCOMUtils.defineLazyServiceGetters(this, { BrowserHandler: ["@mozilla.org/browser/clh;1", "nsIBrowserHandler"], @@ -1716,14 +1717,6 @@ class _ASRouter { } } - // Windows specific calls to write attribution data - // Used by `forceAttribution` to set required targeting attributes for - // RTAMO messages. This should only be called from within about:newtab#asrouter - /* istanbul ignore next */ - async _writeAttributionFile(data) { - await AttributionCode.writeAttributionFile(data); - } - /** * forceAttribution - this function should only be called from within about:newtab#asrouter. * It forces the browser attribution to be set to something specified in asrouter admin @@ -1738,9 +1731,11 @@ class _ASRouter { .join("&"); if (AppConstants.platform === "win") { // The whole attribution data is encoded (again) for windows - this._writeAttributionFile(encodeURIComponent(attributionData)); + await AttributionCode.writeAttributionFile( + encodeURIComponent(attributionData) + ); } else if (AppConstants.platform === "macosx") { - let appPath = Services.dirsvc.get("GreD", Ci.nsIFile).parent.parent.path; + let appPath = MacAttribution.applicationPath; let attributionSvc = Cc["@mozilla.org/mac-attribution;1"].getService( Ci.nsIMacAttributionService ); @@ -1750,6 +1745,9 @@ class _ASRouter { // This sets the Attribution to be the referrer attributionSvc.setReferrerUrl(appPath, referrer, true); + + // Delete attribution data file + await AttributionCode.deleteFileAsync(); } // Clear cache call is only possible in a testing environment diff --git a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js index 0534c0831540..a6b66c29d2a5 100644 --- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js +++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js @@ -146,6 +146,8 @@ describe("ASRouter", () => { allowedCodeKeys: ["foo", "bar", "baz"], _clearCache: () => sinon.stub(), getAttrDataAsync: () => Promise.resolve({ content: "addonID" }), + deleteFileAsync: () => Promise.resolve(), + writeAttributionFile: () => Promise.resolve(), }; FakeBookmarkPanelHub = { init: sandbox.stub(), @@ -186,6 +188,7 @@ describe("ASRouter", () => { AttributionCode: fakeAttributionCode, SnippetsTestMessageProvider, PanelTestProvider, + MacAttribution: { applicationPath: "" }, BookmarkPanelHub: FakeBookmarkPanelHub, ToolbarBadgeHub: FakeToolbarBadgeHub, ToolbarPanelHub: FakeToolbarPanelHub, @@ -2846,6 +2849,7 @@ describe("ASRouter", () => { assert.calledWithExactly(Router.forceAttribution, msg.data.data); }); it("should force attribution and update providers", async () => { + sandbox.stub(AppConstants, "platform").value(""); sandbox.stub(Router, "_updateMessageProviders"); sandbox.stub(Router, "loadMessagesFromAllProviders"); sandbox.stub(fakeAttributionCode, "_clearCache"); @@ -2862,13 +2866,13 @@ describe("ASRouter", () => { assert.calledOnce(Router.loadMessagesFromAllProviders); }); it("should double encode on windows", async () => { - sandbox.stub(Router, "_writeAttributionFile"); + sandbox.stub(fakeAttributionCode, "writeAttributionFile"); Router.forceAttribution({ foo: "FOO!", eh: "NOPE", bar: "BAR?" }); assert.notCalled(setReferrerUrl); assert.calledWithMatch( - Router._writeAttributionFile, + fakeAttributionCode.writeAttributionFile, "foo%3DFOO!%26bar%3DBAR%253F" ); });