From 7efb1350323f4bccda49c137d80f6666ec7c150d Mon Sep 17 00:00:00 2001 From: Olivier Yiptong Date: Wed, 27 Jan 2016 13:50:18 -0500 Subject: [PATCH] Bug 1240169 - aboutNewTabService relies on AboutRedirector for default URL resolution r=mconley --HG-- extra : commitid : LGNcbCje6o4 extra : rebase_source : 0476c10707a52f5a559911f01a37cec1fe6cd171 --- browser/base/content/test/newtab/browser.ini | 3 - .../browser_newtab_external_resource.js | 74 ---------- .../content/test/newtab/external_newtab.html | 11 -- browser/components/about/AboutRedirector.cpp | 3 +- .../test/browser/browser_ext_tabs_create.js | 4 +- .../newtab/NewTabComponents.manifest | 4 +- browser/components/newtab/NewTabURL.jsm | 2 +- .../components/newtab/aboutNewTabService.js | 85 +++++++++-- .../newtab/nsIAboutNewTabService.idl | 7 +- .../newtab/tests/browser/browser.ini | 1 + .../tests/browser/browser_newtab_overrides.js | 139 ++++++++++++++++++ .../browser/browser_remotenewtab_pageloads.js | 56 +++---- .../tests/xpcshell/test_AboutNewTabService.js | 76 +++++++--- .../newtab/tests/xpcshell/test_NewTabURL.js | 7 +- 14 files changed, 310 insertions(+), 162 deletions(-) delete mode 100644 browser/base/content/test/newtab/browser_newtab_external_resource.js delete mode 100644 browser/base/content/test/newtab/external_newtab.html create mode 100644 browser/components/newtab/tests/browser/browser_newtab_overrides.js diff --git a/browser/base/content/test/newtab/browser.ini b/browser/base/content/test/newtab/browser.ini index 71767fd43b21..d0f2a030561a 100644 --- a/browser/base/content/test/newtab/browser.ini +++ b/browser/base/content/test/newtab/browser.ini @@ -47,6 +47,3 @@ support-files = [browser_newtab_bug1178586.js] [browser_newtab_bug1194895.js] [browser_newtab_1188015.js] -[browser_newtab_external_resource.js] -support-files = - external_newtab.html diff --git a/browser/base/content/test/newtab/browser_newtab_external_resource.js b/browser/base/content/test/newtab/browser_newtab_external_resource.js deleted file mode 100644 index cc7edc631649..000000000000 --- a/browser/base/content/test/newtab/browser_newtab_external_resource.js +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Description of the Tests for - * - Bug 1204983 - Allow about: pages to load remote content - * - * We perform two tests: - * (1) We load a new tab (about:newtab) using the default url and make sure that URL - * of the doucment matches about:newtab and the principal is the systemPrincipal. - * (2) We load a new tab (about:newtab) and make sure that document.location as well - * as the nodePrincipal match the URL in the URL bar. - */ - -/* globals Cc, Ci, ok, is, content, TestRunner, addNewTabPageTab, gWindow, Services, info */ -/* exported runTests */ - -"use strict"; - -var aboutNewTabService = Cc["@mozilla.org/browser/aboutnewtab-service;1"] - .getService(Ci.nsIAboutNewTabService); - -const ABOUT_NEWTAB_URI = "about:newtab"; -const PREF_URI = "http://example.com/browser/browser/base/content/test/newtab/external_newtab.html"; -const DEFAULT_URI = aboutNewTabService.newTabURL; - -function* loadNewPageAndVerify(browser, uri) { - let browserLoadedPromise = BrowserTestUtils.waitForEvent(browser, "load", true); - browser.loadURI("about:newtab"); - yield browserLoadedPromise; - - yield ContentTask.spawn(gBrowser.selectedBrowser, { uri: uri }, function* (args) { - let uri = args.uri; - - is(String(content.document.location), uri, "document.location should match " + uri); - is(content.document.documentURI, uri, "document.documentURI should match " + uri); - - if (uri == "about:newtab") { - is(content.document.nodePrincipal, - Services.scriptSecurityManager.getSystemPrincipal(), - "nodePrincipal should match systemPrincipal"); - } - else { - is(content.document.nodePrincipal.URI.spec, uri, - "nodePrincipal should match " + uri); - } - }, true); -} - -add_task(function* () { - // test the default behavior - yield* addNewTabPageTab(); - - let browser = gBrowser.selectedBrowser; - - ok(!aboutNewTabService.overridden, - "sanity check: default URL for about:newtab should not be overriden"); - - yield* loadNewPageAndVerify(browser, ABOUT_NEWTAB_URI); - - // set the pref for about:newtab to point to an exteranl resource - aboutNewTabService.newTabURL = PREF_URI; - ok(aboutNewTabService.overridden, - "sanity check: default URL for about:newtab should be overriden"); - is(aboutNewTabService.newTabURL, PREF_URI, - "sanity check: default URL for about:newtab should return the new URL"); - - yield* loadNewPageAndVerify(browser, PREF_URI); - - // reset to about:newtab and perform sanity check - aboutNewTabService.resetNewTabURL(); - is(aboutNewTabService.newTabURL, DEFAULT_URI, - "sanity check: resetting the URL to about:newtab should return about:newtab"); - - // remove the tab and move on - gBrowser.removeCurrentTab(); -}); diff --git a/browser/base/content/test/newtab/external_newtab.html b/browser/base/content/test/newtab/external_newtab.html deleted file mode 100644 index 73f6eaaa84a7..000000000000 --- a/browser/base/content/test/newtab/external_newtab.html +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - Testpage for bug 1204983 - - - Just a testpage for Bug 1204983
- - diff --git a/browser/components/about/AboutRedirector.cpp b/browser/components/about/AboutRedirector.cpp index 184875fe132c..c62d7da98bdb 100644 --- a/browser/components/about/AboutRedirector.cpp +++ b/browser/components/about/AboutRedirector.cpp @@ -162,7 +162,8 @@ AboutRedirector::NewChannel(nsIURI* aURI, // let the aboutNewTabService decide where to redirect nsCOMPtr aboutNewTabService = do_GetService("@mozilla.org/browser/aboutnewtab-service;1", &rv); - rv = aboutNewTabService->GetNewTabURL(url); + NS_ENSURE_SUCCESS(rv, rv); + rv = aboutNewTabService->GetDefaultURL(url); NS_ENSURE_SUCCESS(rv, rv); } // fall back to the specified url in the map diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_create.js b/browser/components/extensions/test/browser/browser_ext_tabs_create.js index 744d28fa55ec..c5b7ec8c64be 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_create.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_create.js @@ -51,7 +51,7 @@ add_task(function* () { windowId: activeWindow, active: true, pinned: false, - url: "chrome://browser/content/newtab/newTab.xhtml", + url: "about:newtab", }; let tests = [ @@ -65,7 +65,7 @@ add_task(function* () { }, { create: {}, - result: { url: "chrome://browser/content/newtab/newTab.xhtml" }, + result: { url: "about:newtab" }, }, { create: { active: false }, diff --git a/browser/components/newtab/NewTabComponents.manifest b/browser/components/newtab/NewTabComponents.manifest index c2f329ac163e..42db65acd6f0 100644 --- a/browser/components/newtab/NewTabComponents.manifest +++ b/browser/components/newtab/NewTabComponents.manifest @@ -1,2 +1,2 @@ -component {cef25b06-0ef6-4c50-a243-e69f943ef23d} aboutNewTabService.js -contract @mozilla.org/browser/aboutnewtab-service;1 {cef25b06-0ef6-4c50-a243-e69f943ef23d} +component {dfcd2adc-7867-4d3a-ba70-17501f208142} aboutNewTabService.js +contract @mozilla.org/browser/aboutnewtab-service;1 {dfcd2adc-7867-4d3a-ba70-17501f208142} diff --git a/browser/components/newtab/NewTabURL.jsm b/browser/components/newtab/NewTabURL.jsm index f0f5f9633df1..5000eae2e9ae 100644 --- a/browser/components/newtab/NewTabURL.jsm +++ b/browser/components/newtab/NewTabURL.jsm @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/* globals XPCOMUtils, Deprecated, aboutNewTabService*/ +/* globals XPCOMUtils, aboutNewTabService*/ /* exported NewTabURL */ "use strict"; diff --git a/browser/components/newtab/aboutNewTabService.js b/browser/components/newtab/aboutNewTabService.js index f680a2f7db22..cf71311b26db 100644 --- a/browser/components/newtab/aboutNewTabService.js +++ b/browser/components/newtab/aboutNewTabService.js @@ -26,6 +26,8 @@ const LOCAL_NEWTAB_URL = "chrome://browser/content/newtab/newTab.xhtml"; const REMOTE_NEWTAB_URL = "https://newtab.cdn.mozilla.net/" + "v%VERSION%/%CHANNEL%/%LOCALE%/index.html"; +const ABOUT_URL = "about:newtab"; + // Pref that tells if remote newtab is enabled const PREF_REMOTE_ENABLED = "browser.newtabpage.remote"; @@ -46,25 +48,65 @@ function AboutNewTabService() { this.toggleRemote(Services.prefs.getBoolPref(PREF_REMOTE_ENABLED)); } +/* + * A service that allows for the overriding, at runtime, of the newtab page's url. + * Additionally, the service manages pref state between a remote and local newtab page. + * + * There is tight coupling with browser/about/AboutRedirector.cpp. + * + * 1. Browser chrome access: + * + * When the user issues a command to open a new tab page, usually clicking a button + * in the browser chrome or using shortcut keys, the browser chrome code invokes the + * service to obtain the newtab URL. It then loads that URL in a new tab. + * + * When not overridden, the default URL emitted by the service is "about:newtab". + * When overridden, it returns the overriden URL. + * + * 2. Redirector Access: + * + * When the URL loaded is about:newtab, the default behavior, or when entered in the + * URL bar, the redirector is hit. The service is then called to return either of + * two URLs, a chrome or remote one, based on the browser.newtabpage.remote pref. + * + * NOTE: "about:newtab" will always result in a default newtab page, and never an overridden URL. + * + * Access patterns: + * + * The behavior is different when accessing the service via browser chrome or via redirector + * largely to maintain compatibility with expectations of add-on developers. + * + * Loading a chrome resource, or an about: URL in the redirector with either the + * LOAD_NORMAL or LOAD_REPLACE flags yield unexpected behaviors, so a roundtrip + * to the redirector from browser chrome is avoided. + */ AboutNewTabService.prototype = { - _newTabURL: LOCAL_NEWTAB_URL, + _newTabURL: ABOUT_URL, _remoteEnabled: false, + _remoteURL: null, _overridden: false, - classID: Components.ID("{cef25b06-0ef6-4c50-a243-e69f943ef23d}"), + classID: Components.ID("{dfcd2adc-7867-4d3a-ba70-17501f208142}"), QueryInterface: XPCOMUtils.generateQI([Ci.nsIAboutNewTabService]), _xpcom_categories: [{ service: true }], _handleToggleEvent(prefName, stateEnabled, forceState) { //jshint unused:false - this.toggleRemote(stateEnabled, forceState); + if (this.toggleRemote(stateEnabled, forceState)) { + Services.obs.notifyObservers(null, "newtab-url-changed", ABOUT_URL); + } }, /** - * React to changes to the remote newtab pref. Only act - * if there is a change of state and if not overridden. + * React to changes to the remote newtab pref. + * + * If browser.newtabpage.remote is true, this will change the default URL to the + * remote newtab page URL. If browser.newtabpage.remote is false, the default URL + * will be a local chrome URL. + * + * This will only act if there is a change of state and if not overridden. * * @returns {Boolean} Returns if there has been a state change * @@ -79,7 +121,7 @@ AboutNewTabService.prototype = { } if (stateEnabled) { - this._newTabURL = this.generateRemoteURL(); + this._remoteURL = this.generateRemoteURL(); NewTabPrefsProvider.prefs.on( PREF_SELECTED_LOCALE, this._updateRemoteMaybe.bind(this)); @@ -88,11 +130,11 @@ AboutNewTabService.prototype = { this._updateRemoteMaybe.bind(this)); this._remoteEnabled = true; } else { - this._newTabURL = LOCAL_NEWTAB_URL; NewTabPrefsProvider.prefs.off(PREF_SELECTED_LOCALE, this._updateRemoteMaybe); NewTabPrefsProvider.prefs.off(PREF_MATCH_OS_LOCALE, this._updateRemoteMaybe); this._remoteEnabled = false; } + this._newTabURL = ABOUT_URL; return true; }, @@ -108,6 +150,21 @@ AboutNewTabService.prototype = { return url; }, + /* + * Returns the default URL. + * + * This URL only depends on the browser.newtabpage.remote pref. Overriding + * the newtab page has no effect on the result of this function. + * + * @returns {String} the default newtab URL, remote or local depending on browser.newtabpage.remote + */ + get defaultURL() { + if (this._remoteEnabled) { + return this._remoteURL; + } + return LOCAL_NEWTAB_URL; + }, + /* * Updates the remote location when the page is not overriden. * @@ -119,17 +176,17 @@ AboutNewTabService.prototype = { } let url = this.generateRemoteURL(); - if (url !== this._newTabURL) { - this._newTabURL = url; + if (url !== this._remoteURL) { + this._remoteURL = url; Services.obs.notifyObservers(null, "newtab-url-changed", - this._newTabURL); + this._remoteURL); } }, /** * Returns the release name from an Update Channel name * - * @return {String} a release name based on the update channel. Defaults to nightly + * @returns {String} a release name based on the update channel. Defaults to nightly */ releaseFromUpdateChannel(channelName) { return VALID_CHANNELS.has(channelName) ? channelName : "nightly"; @@ -148,10 +205,13 @@ AboutNewTabService.prototype = { }, set newTabURL(aNewTabURL) { - if (aNewTabURL === "about:newtab") { + aNewTabURL = aNewTabURL.trim(); + if (aNewTabURL === ABOUT_URL) { // avoid infinite redirects in case one sets the URL to about:newtab this.resetNewTabURL(); return; + } else if (aNewTabURL === "") { + aNewTabURL = "about:blank"; } let remoteURL = this.generateRemoteURL(); let prefRemoteEnabled = Services.prefs.getBoolPref(PREF_REMOTE_ENABLED); @@ -182,6 +242,7 @@ AboutNewTabService.prototype = { resetNewTabURL() { this._overridden = false; + this._newTabURL = ABOUT_URL; this.toggleRemote(Services.prefs.getBoolPref(PREF_REMOTE_ENABLED), true); Services.obs.notifyObservers(null, "newtab-url-changed", this._newTabURL); } diff --git a/browser/components/newtab/nsIAboutNewTabService.idl b/browser/components/newtab/nsIAboutNewTabService.idl index 5bfc9ab34db7..bc25c7492a72 100644 --- a/browser/components/newtab/nsIAboutNewTabService.idl +++ b/browser/components/newtab/nsIAboutNewTabService.idl @@ -9,7 +9,7 @@ * than the one specified within AboutRedirector.cpp */ -[scriptable, uuid(cef25b06-0ef6-4c50-a243-e69f943ef23d)] +[scriptable, uuid(dfcd2adc-7867-4d3a-ba70-17501f208142)] interface nsIAboutNewTabService : nsISupports { /** @@ -18,6 +18,11 @@ interface nsIAboutNewTabService : nsISupports */ attribute ACString newTabURL; + /** + * Returns the default URL (remote or local depending on pref) + */ + attribute ACString defaultURL; + /** * Returns true if the default resource got overridden. */ diff --git a/browser/components/newtab/tests/browser/browser.ini b/browser/components/newtab/tests/browser/browser.ini index 8ec39550ccff..637f1a46d32a 100644 --- a/browser/components/newtab/tests/browser/browser.ini +++ b/browser/components/newtab/tests/browser/browser.ini @@ -3,3 +3,4 @@ support-files = dummy_page.html [browser_remotenewtab_pageloads.js] +[browser_newtab_overrides.js] diff --git a/browser/components/newtab/tests/browser/browser_newtab_overrides.js b/browser/components/newtab/tests/browser/browser_newtab_overrides.js new file mode 100644 index 000000000000..bbe24e64096b --- /dev/null +++ b/browser/components/newtab/tests/browser/browser_newtab_overrides.js @@ -0,0 +1,139 @@ +/*globals + XPCOMUtils, + aboutNewTabService, + Services, + ContentTask, + TestUtils, + BrowserOpenTab, + registerCleanupFunction, + is, + content +*/ + +"use strict"; + +let Cu = Components.utils; +Cu.import("resource://gre/modules/Task.jsm"); +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://gre/modules/Preferences.jsm"); + +XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService", + "@mozilla.org/browser/aboutnewtab-service;1", + "nsIAboutNewTabService"); + +registerCleanupFunction(function() { + Services.prefs.setBoolPref("browser.newtabpage.remote", false); + aboutNewTabService.resetNewTabURL(); +}); + +/* + * Tests that the default newtab page is always returned when one types "about:newtab" in the URL bar, + * even when overridden. + */ +add_task(function* redirector_ignores_override() { + let overrides = [ + "chrome://browser/content/downloads/contentAreaDownloadsView.xul", + "about:home", + ]; + + for (let overrideURL of overrides) { + let notificationPromise = nextChangeNotificationPromise(overrideURL, `newtab page now points to ${overrideURL}`); + aboutNewTabService.newTabURL = overrideURL; + + yield notificationPromise; + Assert.ok(aboutNewTabService.overridden, "url has been overridden"); + + let tabOptions = { + gBrowser, + url: "about:newtab", + }; + + /* + * Simulate typing "about:newtab" in the url bar. + * + * Bug 1240169 - We expect the redirector to lead the user to "about:newtab", the default URL, + * due to invoking AboutRedirector. A user interacting with the chrome otherwise would lead + * to the overriding URLs. + */ + yield BrowserTestUtils.withNewTab(tabOptions, function*(browser) { + yield ContentTask.spawn(browser, {}, function*() { + is(content.location.href, "about:newtab", "Got right URL"); + is(content.document.location.href, "about:newtab", "Got right URL"); + is(content.document.nodePrincipal, + Services.scriptSecurityManager.getSystemPrincipal(), + "nodePrincipal should match systemPrincipal"); + }); + }); // jshint ignore:line + } +}); + +/* + * Tests loading an overridden newtab page by simulating opening a newtab page from chrome + */ +add_task(function* override_loads_in_browser() { + let overrides = [ + "chrome://browser/content/downloads/contentAreaDownloadsView.xul", + "about:home", + " about:home", + ]; + + for (let overrideURL of overrides) { + let notificationPromise = nextChangeNotificationPromise(overrideURL.trim(), `newtab page now points to ${overrideURL}`); + aboutNewTabService.newTabURL = overrideURL; + + yield notificationPromise; + Assert.ok(aboutNewTabService.overridden, "url has been overridden"); + + // simulate a newtab open as a user would + BrowserOpenTab(); // jshint ignore:line + + let browser = gBrowser.selectedBrowser; + yield BrowserTestUtils.browserLoaded(browser); + + yield ContentTask.spawn(browser, {url: overrideURL}, function*(args) { + is(content.location.href, args.url.trim(), "Got right URL"); + is(content.document.location.href, args.url.trim(), "Got right URL"); + }); // jshint ignore:line + yield BrowserTestUtils.removeTab(gBrowser.selectedTab); + } +}); + +/* + * Tests edge cases when someone overrides the newtabpage with whitespace + */ +add_task(function* override_blank_loads_in_browser() { + let overrides = [ + "", + " ", + "\n\t", + " about:blank", + ]; + + for (let overrideURL of overrides) { + let notificationPromise = nextChangeNotificationPromise("about:blank", "newtab page now points to about:blank"); + aboutNewTabService.newTabURL = overrideURL; + + yield notificationPromise; + Assert.ok(aboutNewTabService.overridden, "url has been overridden"); + + // simulate a newtab open as a user would + BrowserOpenTab(); // jshint ignore:line + + let browser = gBrowser.selectedBrowser; + yield BrowserTestUtils.browserLoaded(browser); + + yield ContentTask.spawn(browser, {}, function*() { + is(content.location.href, "about:blank", "Got right URL"); + is(content.document.location.href, "about:blank", "Got right URL"); + }); // jshint ignore:line + yield BrowserTestUtils.removeTab(gBrowser.selectedTab); + } +}); + +function nextChangeNotificationPromise(aNewURL, testMessage) { + return TestUtils.topicObserved("newtab-url-changed", function observer(aSubject, aData) { // jshint unused:false + Assert.equal(aData, aNewURL, testMessage); + return true; + }.bind(this)); +} diff --git a/browser/components/newtab/tests/browser/browser_remotenewtab_pageloads.js b/browser/components/newtab/tests/browser/browser_remotenewtab_pageloads.js index 1b397b3afe64..7505b0034f35 100644 --- a/browser/components/newtab/tests/browser/browser_remotenewtab_pageloads.js +++ b/browser/components/newtab/tests/browser/browser_remotenewtab_pageloads.js @@ -1,19 +1,18 @@ -/* globals XPCOMUtils, aboutNewTabService, Services */ +/* globals Cu, XPCOMUtils, TestUtils, aboutNewTabService, ContentTask, content, is */ "use strict"; -let Cu = Components.utils; Cu.import("resource://gre/modules/Task.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm"); -Cu.import("resource://gre/modules/Services.jsm"); -XPCOMUtils.defineLazyModuleGetter(this, "RemotePageManager", - "resource://gre/modules/RemotePageManager.jsm"); XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService", "@mozilla.org/browser/aboutnewtab-service;1", "nsIAboutNewTabService"); const TEST_URL = "https://example.com/browser/browser/components/newtab/tests/browser/dummy_page.html"; +/* + * Tests opening a newtab page with a remote URL. Simulates a newtab open from chrome + */ add_task(function* open_newtab() { let notificationPromise = nextChangeNotificationPromise(TEST_URL, "newtab page now points to test url"); aboutNewTabService.newTabURL = TEST_URL; @@ -21,37 +20,30 @@ add_task(function* open_newtab() { yield notificationPromise; Assert.ok(aboutNewTabService.overridden, "url has been overridden"); - let tabOptions = { - gBrowser, - url: "about:newtab", - }; + /* + * Simulate a newtab open as a user would. + * + * Bug 1240169 - We cannot set the URL to about:newtab because that would invoke the redirector. + * The redirector always yields the loading of a default newtab URL. We expect the user to use + * the browser UI to access overriding URLs, for istance by click on the "+" button in the tab + * bar, or by using the new tab shortcut key. + */ + BrowserOpenTab(); // jshint ignore:line - yield BrowserTestUtils.withNewTab(tabOptions, function* (browser) { - Assert.equal(TEST_URL, browser.contentWindow.location, `New tab should open to ${TEST_URL}`); - }); -}); - -add_task(function* emptyURL() { - let notificationPromise = nextChangeNotificationPromise("", "newtab service now points to empty url"); - aboutNewTabService.newTabURL = ""; - yield notificationPromise; - - let tabOptions = { - gBrowser, - url: "about:newtab", - }; - - yield BrowserTestUtils.withNewTab(tabOptions, function* (browser) { - Assert.equal("about:blank", browser.contentWindow.location, `New tab should open to ${"about:blank"}`); + let browser = gBrowser.selectedBrowser; + yield BrowserTestUtils.browserLoaded(browser); + + yield ContentTask.spawn(browser, {url: TEST_URL}, function*(args) { + is(content.document.location.href, args.url, "document.location should match the external resource"); + is(content.document.documentURI, args.url, "document.documentURI should match the external resource"); + is(content.document.nodePrincipal.URI.spec, args.url, "nodePrincipal should match the external resource"); }); + yield BrowserTestUtils.removeTab(gBrowser.selectedTab); }); function nextChangeNotificationPromise(aNewURL, testMessage) { - return new Promise(resolve => { - Services.obs.addObserver(function observer(aSubject, aTopic, aData) { // jshint unused:false - Services.obs.removeObserver(observer, aTopic); + return TestUtils.topicObserved("newtab-url-changed", function observer(aSubject, aData) { // jshint unused:false Assert.equal(aData, aNewURL, testMessage); - resolve(); - }, "newtab-url-changed", false); - }); + return true; + }.bind(this)); } diff --git a/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js b/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js index 265132c842a8..9e7510c15b9a 100644 --- a/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js +++ b/browser/components/newtab/tests/xpcshell/test_AboutNewTabService.js @@ -2,7 +2,7 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ -/* globals Services, XPCOMUtils, NewTabPrefsProvider, Preferences, aboutNewTabService */ +/* globals Services, XPCOMUtils, NewTabPrefsProvider, Preferences, aboutNewTabService, do_register_cleanup */ "use strict"; @@ -19,25 +19,30 @@ XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService", "nsIAboutNewTabService"); const DEFAULT_HREF = aboutNewTabService.generateRemoteURL(); +const DEFAULT_CHROME_URL = "chrome://browser/content/newtab/newTab.xhtml"; +const DOWNLOADS_URL = "chrome://browser/content/downloads/contentAreaDownloadsView.xul"; + +function cleanup() { + Services.prefs.setBoolPref("browser.newtabpage.remote", false); + aboutNewTabService.resetNewTabURL(); + NewTabPrefsProvider.prefs.uninit(); +} + +do_register_cleanup(cleanup); /** * Test the overriding of the default URL */ -add_task(function* () { +add_task(function* test_override_remote_disabled() { NewTabPrefsProvider.prefs.init(); let notificationPromise; Services.prefs.setBoolPref("browser.newtabpage.remote", false); - let localChromeURL = "chrome://browser/content/newtab/newTab.xhtml"; // tests default is the local newtab resource - Assert.equal(aboutNewTabService.newTabURL, localChromeURL, - `Default newtab URL should be ${localChromeURL}`); - - // test the newtab service does not go in a circular redirect - aboutNewTabService.newTabURL = "about:newtab"; - Assert.equal(aboutNewTabService.newTabURL, localChromeURL, - "Newtab URL avoids a circular redirect by setting to the default URL"); + Assert.equal(aboutNewTabService.defaultURL, DEFAULT_CHROME_URL, + `Default newtab URL should be ${DEFAULT_CHROME_URL}`); + // override with some remote URL let url = "http://example.com/"; notificationPromise = nextChangeNotificationPromise(url); aboutNewTabService.newTabURL = url; @@ -46,27 +51,59 @@ add_task(function* () { Assert.ok(!aboutNewTabService.remoteEnabled, "Newtab remote should not be enabled"); Assert.equal(aboutNewTabService.newTabURL, url, "Newtab URL should be the custom URL"); - notificationPromise = nextChangeNotificationPromise("chrome://browser/content/newtab/newTab.xhtml"); + // test reset with remote disabled + notificationPromise = nextChangeNotificationPromise("about:newtab"); aboutNewTabService.resetNewTabURL(); yield notificationPromise; Assert.ok(!aboutNewTabService.overridden, "Newtab URL should not be overridden"); - Assert.equal(aboutNewTabService.newTabURL, "chrome://browser/content/newtab/newTab.xhtml", - "Newtab URL should be the default"); + Assert.equal(aboutNewTabService.newTabURL, "about:newtab", "Newtab URL should be the default"); + // test override to a chrome URL + notificationPromise = nextChangeNotificationPromise(DOWNLOADS_URL); + aboutNewTabService.newTabURL = DOWNLOADS_URL; + yield notificationPromise; + Assert.ok(aboutNewTabService.overridden, "Newtab URL should be overridden"); + Assert.equal(aboutNewTabService.newTabURL, DOWNLOADS_URL, "Newtab URL should be the custom URL"); + + cleanup(); +}); + +add_task(function* test_override_remote_enabled() { + NewTabPrefsProvider.prefs.init(); + let notificationPromise; // change newtab page to remote + notificationPromise = nextChangeNotificationPromise("about:newtab"); Services.prefs.setBoolPref("browser.newtabpage.remote", true); + yield notificationPromise; let remoteHref = aboutNewTabService.generateRemoteURL(); - Assert.equal(aboutNewTabService.newTabURL, remoteHref, "Newtab URL should be the default remote URL"); + Assert.equal(aboutNewTabService.defaultURL, remoteHref, "Newtab URL should be the default remote URL"); Assert.ok(!aboutNewTabService.overridden, "Newtab URL should not be overridden"); Assert.ok(aboutNewTabService.remoteEnabled, "Newtab remote should be enabled"); - NewTabPrefsProvider.prefs.uninit(); + + // change to local newtab page while remote is enabled + notificationPromise = nextChangeNotificationPromise(DEFAULT_CHROME_URL); + aboutNewTabService.newTabURL = DEFAULT_CHROME_URL; + yield notificationPromise; + Assert.equal(aboutNewTabService.newTabURL, DEFAULT_CHROME_URL, + "Newtab URL set to chrome url"); + Assert.equal(aboutNewTabService.defaultURL, DEFAULT_CHROME_URL, + "Newtab URL defaultURL set to the default chrome URL"); + Assert.ok(aboutNewTabService.overridden, "Newtab URL should be overridden"); + Assert.ok(!aboutNewTabService.remoteEnabled, "Newtab remote should not be enabled"); + + cleanup(); }); /** * Tests reponse to updates to prefs */ add_task(function* test_updates() { + /* + * Simulates a "cold-boot" situation, with some pref already set before testing a series + * of changes. + */ Preferences.set("browser.newtabpage.remote", true); + aboutNewTabService.resetNewTabURL(); // need to set manually because pref notifs are off let notificationPromise; let expectedHref = "https://newtab.cdn.mozilla.net" + `/v${aboutNewTabService.remoteVersion}` + @@ -98,9 +135,10 @@ add_task(function* test_updates() { // from overridden to default notificationPromise = nextChangeNotificationPromise( - DEFAULT_HREF, "a notification occurs on reset"); + "about:newtab", "a notification occurs on reset"); aboutNewTabService.resetNewTabURL(); Assert.ok(aboutNewTabService.remoteEnabled, "Newtab remote should be enabled"); + Assert.equal(aboutNewTabService.defaultURL, DEFAULT_HREF, "Default URL should be the remote page"); yield notificationPromise; // override to default URL from default URL @@ -109,16 +147,16 @@ add_task(function* test_updates() { aboutNewTabService.newTabURL = aboutNewTabService.generateRemoteURL(); Assert.ok(aboutNewTabService.remoteEnabled, "Newtab remote should be enabled"); aboutNewTabService.newTabURL = testURL; - Assert.ok(!aboutNewTabService.remoteEnabled, "Newtab remote should not be enabled"); yield notificationPromise; + Assert.ok(!aboutNewTabService.remoteEnabled, "Newtab remote should not be enabled"); // reset twice, only one notification for default URL notificationPromise = nextChangeNotificationPromise( - DEFAULT_HREF, "reset occurs"); + "about:newtab", "reset occurs"); aboutNewTabService.resetNewTabURL(); yield notificationPromise; - NewTabPrefsProvider.prefs.uninit(); + cleanup(); }); /** diff --git a/browser/components/newtab/tests/xpcshell/test_NewTabURL.js b/browser/components/newtab/tests/xpcshell/test_NewTabURL.js index 91ef041248b2..1505e638cc75 100644 --- a/browser/components/newtab/tests/xpcshell/test_NewTabURL.js +++ b/browser/components/newtab/tests/xpcshell/test_NewTabURL.js @@ -2,7 +2,7 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ -/* globals Services, NewTabURL, XPCOMUtils, aboutNewTabService */ +/* globals Services, NewTabURL, XPCOMUtils, aboutNewTabService, NewTabPrefsProvider */ "use strict"; const {utils: Cu} = Components; @@ -15,7 +15,7 @@ XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService", "@mozilla.org/browser/aboutnewtab-service;1", "nsIAboutNewTabService"); -add_task(function* () { +add_task(function*() { let defaultURL = aboutNewTabService.newTabURL; Services.prefs.setBoolPref("browser.newtabpage.remote", false); @@ -36,8 +36,7 @@ add_task(function* () { // change newtab page to remote NewTabPrefsProvider.prefs.init(); Services.prefs.setBoolPref("browser.newtabpage.remote", true); - let remoteURL = aboutNewTabService.generateRemoteURL(); - Assert.equal(NewTabURL.get(), remoteURL, `Newtab URL should be ${remoteURL}`); + Assert.equal(NewTabURL.get(), "about:newtab", `Newtab URL should be about:newtab`); Assert.ok(!NewTabURL.overridden, "Newtab URL should not be overridden"); NewTabPrefsProvider.prefs.uninit(); });