From ce4ad5b71ba758d5224f967ecc4e9c757464111c Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Mon, 10 Mar 2014 18:33:00 -0700 Subject: [PATCH] Bug 956605 - have about:accounts do the right thing based on user logged in states. r=ckarlof --- .../content/aboutaccounts/aboutaccounts.js | 93 ++++++++--- .../test/general/browser_aboutAccounts.js | 148 ++++++++++++++++-- .../test/general/content_aboutAccounts.js | 32 +++- 3 files changed, 240 insertions(+), 33 deletions(-) diff --git a/browser/base/content/aboutaccounts/aboutaccounts.js b/browser/base/content/aboutaccounts/aboutaccounts.js index bf6fc6cf5157..641267462cf5 100644 --- a/browser/base/content/aboutaccounts/aboutaccounts.js +++ b/browser/base/content/aboutaccounts/aboutaccounts.js @@ -9,9 +9,17 @@ const {classes: Cc, interfaces: Ci, utils: Cu} = Components; Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/FxAccounts.jsm"); +let fxAccountsCommon = {}; +Cu.import("resource://gre/modules/FxAccountsCommon.js", fxAccountsCommon); + const PREF_LAST_FXA_USER = "identity.fxaccounts.lastSignedInUserHash"; const PREF_SYNC_SHOW_CUSTOMIZATION = "services.sync.ui.showCustomizationDialog"; +const OBSERVER_TOPICS = [ + fxAccountsCommon.ONVERIFIED_NOTIFICATION, + fxAccountsCommon.ONLOGOUT_NOTIFICATION, +]; + function log(msg) { //dump("FXA: " + msg + "\n"); }; @@ -150,6 +158,11 @@ let wrapper = { xps.whenLoaded().then(() => { return fxAccounts.setSignedInUser(accountData); }).then(() => { + // If the user data is verified, we want it to immediately look like + // they are signed in without waiting for messages to bounce around. + if (accountData.verified) { + showManage(); + } this.injectData("message", { status: "login" }); // until we sort out a better UX, just leave the jelly page in place. // If the account email is not yet verified, it will tell the user to @@ -249,23 +262,36 @@ function openPrefs() { } function init() { - if (window.location.href.contains("action=signin")) { - show("remote"); - wrapper.init(fxAccounts.getAccountsSignInURI()); - } else if (window.location.href.contains("action=signup")) { - show("remote"); - wrapper.init(); - } else if (window.location.href.contains("action=reauth")) { - fxAccounts.promiseAccountsForceSigninURI().then(url => { - show("remote"); - wrapper.init(url); - }); - } else { - // Check if we have a local account - fxAccounts.getSignedInUser().then(user => { + fxAccounts.getSignedInUser().then(user => { + if (window.location.href.contains("action=signin")) { if (user) { - show("stage"); - show("manage"); + // asking to sign-in when already signed in just shows manage. + showManage(); + } else { + show("remote"); + wrapper.init(fxAccounts.getAccountsSignInURI()); + } + } else if (window.location.href.contains("action=signup")) { + if (user) { + // asking to sign-up when already signed in just shows manage. + showManage(); + } else { + show("remote"); + wrapper.init(); + } + } else if (window.location.href.contains("action=reauth")) { + // ideally we would only show this when we know the user is in a + // "must reauthenticate" state - but we don't. + // As the email address will be included in the URL returned from + // promiseAccountsForceSigninURI, just always show it. + fxAccounts.promiseAccountsForceSigninURI().then(url => { + show("remote"); + wrapper.init(url); + }); + } else { + // No action specified + if (user) { + showManage(); let sb = Services.strings.createBundle("chrome://browser/locale/syncSetup.properties"); document.title = sb.GetStringFromName("manage.pageTitle"); } else { @@ -274,8 +300,8 @@ function init() { // load the remote frame in the background wrapper.init(); } - }); - } + } + }); } function show(id) { @@ -285,7 +311,38 @@ function hide(id) { document.getElementById(id).style.display = 'none'; } +function showManage() { + show("stage"); + show("manage"); + hide("remote"); + hide("intro"); +} + document.addEventListener("DOMContentLoaded", function onload() { document.removeEventListener("DOMContentLoaded", onload, true); init(); }, true); + +function initObservers() { + function observe(subject, topic, data) { + log("about:accounts observed " + topic); + if (topic == fxAccountsCommon.ONLOGOUT_NOTIFICATION) { + // All about:account windows get changed to action=signin on logout. + window.location = "about:accounts?action=signin"; + return; + } + // must be onverified - just about:accounts is loaded. + window.location = "about:accounts"; + } + + for (let topic of OBSERVER_TOPICS) { + Services.obs.addObserver(observe, topic, false); + } + window.addEventListener("unload", function(event) { + log("about:accounts unloading") + for (let topic of OBSERVER_TOPICS) { + Services.obs.removeObserver(observe, topic); + } + }); +} +initObservers(); diff --git a/browser/base/content/test/general/browser_aboutAccounts.js b/browser/base/content/test/general/browser_aboutAccounts.js index f20874e677f1..a1512479fd1e 100644 --- a/browser/base/content/test/general/browser_aboutAccounts.js +++ b/browser/base/content/test/general/browser_aboutAccounts.js @@ -28,7 +28,10 @@ registerCleanupFunction(function() { let gTests = [ { desc: "Test the remote commands", - teardown: () => gBrowser.removeCurrentTab(), + teardown: function* () { + gBrowser.removeCurrentTab(); + yield fxAccounts.signOut(); + }, run: function* () { setPref("identity.fxaccounts.remote.uri", @@ -57,22 +60,57 @@ let gTests = [ ok(false, "Failed to get all commands"); deferred.reject(); } - return deferred.promise.then(() => fxAccounts.signOut()); + yield deferred.promise; } }, { - desc: "Test action=signin", + desc: "Test action=signin - no user logged in", teardown: () => gBrowser.removeCurrentTab(), run: function* () { + // When this loads with no user logged-in, we expect the "normal" URL const expected_url = "https://example.com/?is_sign_in"; setPref("identity.fxaccounts.remote.signin.uri", expected_url); let [tab, url] = yield promiseNewTabWithIframeLoadEvent("about:accounts?action=signin"); is(url, expected_url, "action=signin got the expected URL"); + // we expect the remote iframe to be shown. + yield checkVisibilities(tab, { + stage: false, // parent of 'manage' and 'intro' + manage: false, + intro: false, // this is "get started" + remote: true + }); } }, { - desc: "Test action=signup", + desc: "Test action=signin - user logged in", + teardown: function* () { + gBrowser.removeCurrentTab(); + yield signOut(); + }, + run: function* () + { + // When this loads with a user logged-in, we expect the normal URL to + // have been ignored and the "manage" page to be shown. + const expected_url = "https://example.com/?is_sign_in"; + setPref("identity.fxaccounts.remote.signin.uri", expected_url); + yield setSignedInUser(); + let tab = yield promiseNewTabLoadEvent("about:accounts?action=signin"); + // about:accounts initializes after fetching the current user from Fxa - + // so we also request it - by the time we get it we know it should have + // done its thing. + yield fxAccounts.getSignedInUser(); + // we expect "manage" to be shown. + yield checkVisibilities(tab, { + stage: true, // parent of 'manage' and 'intro' + manage: true, + intro: false, // this is "get started" + remote: false + }); + } +}, +{ + desc: "Test action=signup - no user logged in", teardown: () => gBrowser.removeCurrentTab(), run: function* () { @@ -80,13 +118,39 @@ let gTests = [ setPref("identity.fxaccounts.remote.uri", expected_url); let [tab, url] = yield promiseNewTabWithIframeLoadEvent("about:accounts?action=signup"); is(url, expected_url, "action=signup got the expected URL"); + // we expect the remote iframe to be shown. + yield checkVisibilities(tab, { + stage: false, // parent of 'manage' and 'intro' + manage: false, + intro: false, // this is "get started" + remote: true + }); + }, +}, +{ + desc: "Test action=signup - user logged in", + teardown: () => gBrowser.removeCurrentTab(), + run: function* () + { + const expected_url = "https://example.com/?is_sign_up"; + setPref("identity.fxaccounts.remote.uri", expected_url); + yield setSignedInUser(); + let tab = yield promiseNewTabLoadEvent("about:accounts?action=signup"); + yield fxAccounts.getSignedInUser(); + // we expect "manage" to be shown. + yield checkVisibilities(tab, { + stage: true, // parent of 'manage' and 'intro' + manage: true, + intro: false, // this is "get started" + remote: false + }); }, }, { desc: "Test action=reauth", teardown: function* () { gBrowser.removeCurrentTab(); - yield fxAccounts.signOut(); + yield signOut(); }, run: function* () { @@ -102,14 +166,29 @@ let gTests = [ verified: true }; - yield fxAccounts.setSignedInUser(userData); + yield setSignedInUser(); let [tab, url] = yield promiseNewTabWithIframeLoadEvent("about:accounts?action=reauth"); // The current user will be appended to the url let expected = expected_url + "&email=foo%40example.com"; is(url, expected, "action=reauth got the expected URL"); }, }, - +{ + desc: "Test observers about:accounts", + teardown: function() { + gBrowser.removeCurrentTab(); + }, + run: function* () { + setPref("identity.fxaccounts.remote.uri", "https://example.com/"); + yield setSignedInUser(); + let tab = yield promiseNewTabLoadEvent("about:accounts"); + // sign the user out - the tab should have action=signin + yield signOut(); + // wait for the new load. + yield promiseOneMessage(tab, "test:document:load"); + is(tab.linkedBrowser.contentDocument.location.href, "about:accounts?action=signin"); + } +}, ]; // gTests function test() @@ -130,9 +209,18 @@ function test() }); } +function promiseOneMessage(tab, messageName) { + let mm = tab.linkedBrowser.messageManager; + let deferred = Promise.defer(); + mm.addMessageListener(messageName, function onmessage(message) { + mm.removeMessageListener(messageName, onmessage); + deferred.resolve(message); + }); + return deferred.promise; +} + function promiseNewTabLoadEvent(aUrl) { - let deferred = Promise.defer(); let tab = gBrowser.selectedTab = gBrowser.addTab(aUrl); let browser = tab.linkedBrowser; let mm = browser.messageManager; @@ -140,11 +228,9 @@ function promiseNewTabLoadEvent(aUrl) // give it an e10s-friendly content script to help with our tests. mm.loadFrameScript(CHROME_BASE + "content_aboutAccounts.js", true); // and wait for it to tell us about the load. - mm.addMessageListener("test:document:load", function onLoad() { - mm.removeMessageListener("test:document:load", onLoad); - deferred.resolve(tab); - }); - return deferred.promise; + return promiseOneMessage(tab, "test:document:load").then( + () => tab + ); } // Returns a promise which is resolved with the iframe's URL after a new @@ -164,3 +250,39 @@ function promiseNewTabWithIframeLoadEvent(aUrl) { }); return deferred.promise; } + +function checkVisibilities(tab, data) { + let ids = Object.keys(data); + let mm = tab.linkedBrowser.messageManager; + let deferred = Promise.defer(); + mm.addMessageListener("test:check-visibilities-response", function onResponse(message) { + mm.removeMessageListener("test:check-visibilities-response", onResponse); + for (let id of ids) { + is(message.data[id], data[id], "Element '" + id + "' has correct visibility"); + } + deferred.resolve(); + }); + mm.sendAsyncMessage("test:check-visibilities", {ids: ids}); + return deferred.promise; +} + +// watch out - these will fire observers which if you aren't careful, may +// interfere with the tests. +function setSignedInUser(data) { + if (!data) { + data = { + email: "foo@example.com", + uid: "1234@lcip.org", + assertion: "foobar", + sessionToken: "dead", + kA: "beef", + kB: "cafe", + verified: true + } + } + return fxAccounts.setSignedInUser(data); +} + +function signOut() { + return fxAccounts.signOut(); +} diff --git a/browser/base/content/test/general/content_aboutAccounts.js b/browser/base/content/test/general/content_aboutAccounts.js index 5221319b9040..e09cdae1aa96 100644 --- a/browser/base/content/test/general/content_aboutAccounts.js +++ b/browser/base/content/test/general/content_aboutAccounts.js @@ -5,9 +5,16 @@ // This file is loaded as a "content script" for browser_aboutAccounts tests "use strict"; -addEventListener("DOMContentLoaded", function domContentLoaded(event) { - removeEventListener("DOMContentLoaded", domContentLoaded); +addEventListener("load", function load(event) { + if (event.target != content.document) { + return; + } +// content.document.removeEventListener("load", load, true); sendAsyncMessage("test:document:load"); +}, true); + +addEventListener("DOMContentLoaded", function domContentLoaded(event) { + removeEventListener("DOMContentLoaded", domContentLoaded, true); let iframe = content.document.getElementById("remote"); iframe.addEventListener("load", function iframeLoaded(event) { if (iframe.contentWindow.location.href == "about:blank" || @@ -17,4 +24,25 @@ addEventListener("DOMContentLoaded", function domContentLoaded(event) { iframe.removeEventListener("load", iframeLoaded, true); sendAsyncMessage("test:iframe:load", {url: iframe.getAttribute("src")}); }, true); +}, true); + +// Return the visibility state of a list of ids. +addMessageListener("test:check-visibilities", function (message) { + let result = {}; + for (let id of message.data.ids) { + let elt = content.document.getElementById(id); + if (elt) { + let displayStyle = content.window.getComputedStyle(elt).display; + if (displayStyle == 'none') { + result[id] = false; + } else if (displayStyle == 'block') { + result[id] = true; + } else { + result[id] = "strange: " + displayStyle; // tests should fail! + } + } else { + result[id] = "doesn't exist: " + id; + } + } + sendAsyncMessage("test:check-visibilities-response", result); });