From 10b05818ac645b09d6135345b8da703629b22e27 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Sat, 31 Jan 2015 18:17:13 -0500 Subject: [PATCH] Bug 1115227 - Loop: add part of the UITour PageID to the Hello tour URLs as a query parameter. r=MattN --- browser/components/loop/MozLoopService.jsm | 18 ++++- browser/components/uitour/UITour.jsm | 17 +++- browser/components/uitour/test/browser.ini | 2 +- .../uitour/test/browser_UITour_loop.js | 79 ++++++++++++++++++- 4 files changed, 108 insertions(+), 8 deletions(-) diff --git a/browser/components/loop/MozLoopService.jsm b/browser/components/loop/MozLoopService.jsm index 572b348a3d8..23b57335075 100644 --- a/browser/components/loop/MozLoopService.jsm +++ b/browser/components/loop/MozLoopService.jsm @@ -1535,13 +1535,29 @@ this.MozLoopService = { let urlStr = this.getLoopPref("gettingStarted.url"); let url = new URL(Services.urlFormatter.formatURL(urlStr)); for (let paramName in aAdditionalParams) { - url.searchParams.append(paramName, aAdditionalParams[paramName]); + url.searchParams.set(paramName, aAdditionalParams[paramName]); } if (aSrc) { url.searchParams.set("utm_source", "firefox-browser"); url.searchParams.set("utm_medium", "firefox-browser"); url.searchParams.set("utm_campaign", aSrc); } + + // Find the most recent pageID that has the Loop prefix. + let mostRecentLoopPageID = {id: null, lastSeen: null}; + for (let pageID of UITour.pageIDsForSession) { + if (pageID[0] && pageID[0].startsWith("hello-tour_OpenPanel_") && + pageID[1] && pageID[1].lastSeen > mostRecentLoopPageID.lastSeen) { + mostRecentLoopPageID.id = pageID[0]; + mostRecentLoopPageID.lastSeen = pageID[1].lastSeen; + } + } + + const PAGE_ID_EXPIRATION_MS = 60 * 60 * 1000; + if (mostRecentLoopPageID.id && + mostRecentLoopPageID.lastSeen > Date.now() - PAGE_ID_EXPIRATION_MS) { + url.searchParams.set("utm_content", mostRecentLoopPageID.id); + } return url; }, diff --git a/browser/components/uitour/UITour.jsm b/browser/components/uitour/UITour.jsm index 9737ded28ea..a9a361e3fae 100644 --- a/browser/components/uitour/UITour.jsm +++ b/browser/components/uitour/UITour.jsm @@ -74,6 +74,9 @@ XPCOMUtils.defineLazyGetter(this, "log", () => { this.UITour = { url: null, seenPageIDs: null, + // This map is not persisted and is used for + // building the content source of a potential tour. + pageIDsForSession: new Map(), pageIDSourceBrowsers: new WeakMap(), /* Map from browser chrome windows to a Set of s in which a tour is open (both visible and hidden) */ tourBrowsersByWindow: new WeakMap(), @@ -375,16 +378,22 @@ this.UITour = { switch (action) { case "registerPageID": { - // This is only relevant if Telemtry is enabled. + if (typeof data.pageID != "string") { + log.warn("registerPageID: pageID must be a string"); + break; + } + + this.pageIDsForSession.set(data.pageID, {lastSeen: Date.now()}); + + // The rest is only relevant if Telemetry is enabled. if (!UITelemetry.enabled) { - log.debug("registerPageID: Telemery disabled, not doing anything"); + log.debug("registerPageID: Telemetry disabled, not doing anything"); break; } // We don't want to allow BrowserUITelemetry.BUCKET_SEPARATOR in the // pageID, as it could make parsing the telemetry bucket name difficult. - if (typeof data.pageID != "string" || - data.pageID.contains(BrowserUITelemetry.BUCKET_SEPARATOR)) { + if (data.pageID.contains(BrowserUITelemetry.BUCKET_SEPARATOR)) { log.warn("registerPageID: Invalid page ID specified"); break; } diff --git a/browser/components/uitour/test/browser.ini b/browser/components/uitour/test/browser.ini index 294f1721008..e86d5da55b2 100644 --- a/browser/components/uitour/test/browser.ini +++ b/browser/components/uitour/test/browser.ini @@ -20,7 +20,7 @@ skip-if = e10s # Bug 941428 - UITour.jsm not e10s friendly. [browser_UITour_heartbeat.js] skip-if = e10s # Bug 941428 - UITour.jsm not e10s friendly. [browser_UITour_loop.js] -skip-if = e10s # Bug 941428 - UITour.jsm not e10s friendly. +skip-if = os == "linux" || e10s # Bug 941428 - UITour.jsm not e10s friendly. [browser_UITour_modalDialog.js] run-if = os == "mac" # modal dialog disabling only working on OS X skip-if = e10s # Bug 941428 - UITour.jsm not e10s friendly diff --git a/browser/components/uitour/test/browser_UITour_loop.js b/browser/components/uitour/test/browser_UITour_loop.js index d3ae6e03493..88c842211d0 100644 --- a/browser/components/uitour/test/browser_UITour_loop.js +++ b/browser/components/uitour/test/browser_UITour_loop.js @@ -27,7 +27,84 @@ function runOffline(fun) { } let tests = [ + taskify(function* test_gettingStartedClicked_linkOpenedWithExpectedParams() { + Services.prefs.setBoolPref("loop.gettingStarted.seen", false); + Services.prefs.setCharPref("loop.gettingStarted.url", "http://example.com"); + ise(loopButton.open, false, "Menu should initially be closed"); + loopButton.click(); + + yield waitForConditionPromise(() => { + return loopButton.open; + }, "Menu should be visible after showMenu()"); + + gContentAPI.registerPageID("hello-tour_OpenPanel_testPage"); + yield new Promise(resolve => { + gContentAPI.ping(() => resolve()); + }); + + let loopDoc = document.getElementById("loop-notification-panel").children[0].contentDocument; + let gettingStartedButton = loopDoc.getElementById("fte-button"); + ok(gettingStartedButton, "Getting Started button should be found"); + + let newTabPromise = waitForConditionPromise(() => { + return gBrowser.currentURI.path.contains("utm_source=firefox-browser"); + }, "New tab with utm_content=testPageNewID should have opened"); + + gettingStartedButton.click(); + yield newTabPromise; + ok(gBrowser.currentURI.path.contains("utm_content=hello-tour_OpenPanel_testPage"), + "Expected URL opened (" + gBrowser.currentURI.path + ")"); + yield gBrowser.removeCurrentTab(); + + checkLoopPanelIsHidden(); + }), + taskify(function* test_gettingStartedClicked_linkOpenedWithExpectedParams2() { + Services.prefs.setBoolPref("loop.gettingStarted.seen", false); + // Force a refresh of the loop panel since going from seen -> unseen doesn't trigger + // automatic re-rendering. + let loopWin = document.getElementById("loop-notification-panel").children[0].contentWindow; + var event = new loopWin.CustomEvent("GettingStartedSeen"); + loopWin.dispatchEvent(event); + + UITour.pageIDsForSession.clear(); + Services.prefs.setCharPref("loop.gettingStarted.url", "http://example.com"); + ise(loopButton.open, false, "Menu should initially be closed"); + loopButton.click(); + + yield waitForConditionPromise(() => { + return loopButton.open; + }, "Menu should be visible after showMenu()"); + + + gContentAPI.registerPageID("hello-tour_OpenPanel_testPageOldId"); + yield new Promise(resolve => { + gContentAPI.ping(() => resolve()); + }); + // Set the time of the page ID to 10 hours earlier, so that it is considered "expired". + UITour.pageIDsForSession.set("hello-tour_OpenPanel_testPageOldId", + {lastSeen: Date.now() - (10 * 60 * 60 * 1000)}); + + let loopDoc = loopWin.document; + let gettingStartedButton = loopDoc.getElementById("fte-button"); + ok(gettingStartedButton, "Getting Started button should be found"); + + let newTabPromise = waitForConditionPromise(() => { + Services.console.logStringMessage(gBrowser.currentURI.path); + return gBrowser.currentURI.path.contains("utm_source=firefox-browser"); + }, "New tab with utm_content=testPageNewID should have opened"); + + gettingStartedButton.click(); + yield newTabPromise; + ok(!gBrowser.currentURI.path.contains("utm_content=hello-tour_OpenPanel_testPageOldId"), + "Expected URL opened without the utm_content parameter (" + + gBrowser.currentURI.path + ")"); + yield gBrowser.removeCurrentTab(); + + checkLoopPanelIsHidden(); + }), taskify(function* test_menu_show_hide() { + // The targets to highlight only appear after getting started is launched. + Services.prefs.setBoolPref("loop.gettingStarted.seen", true); ise(loopButton.open, false, "Menu should initially be closed"); gContentAPI.showMenu("loop"); @@ -277,8 +354,6 @@ function setupFakeRoom() { if (Services.prefs.getBoolPref("loop.enabled")) { loopButton = window.LoopUI.toolbarButton.node; - // The targets to highlight only appear after getting started is launched. - Services.prefs.setBoolPref("loop.gettingStarted.seen", true); registerCleanupFunction(() => { Services.prefs.clearUserPref("loop.gettingStarted.resumeOnFirstJoin");