From b93f2c2970f311aecb05d461b902dcb053b037df Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Fri, 27 Sep 2019 14:54:38 +0000 Subject: [PATCH] Bug 1576158 - Display the private browsing engine on the private browsing page. r=Mardak Differential Revision: https://phabricator.services.mozilla.com/D47208 --HG-- extra : moz-landing-system : lando --- browser/base/content/contentSearchUI.js | 27 +++++++- .../about/AboutPrivateBrowsingHandler.jsm | 3 +- .../content/aboutPrivateBrowsing.js | 4 +- .../browser/browser_privatebrowsing_about.js | 45 +++++++++++++- .../components/search/test/browser/head.js | 12 +++- browser/modules/ContentSearch.jsm | 13 ++-- browser/modules/test/browser/browser.ini | 1 + .../test/browser/browser_ContentSearch.js | 62 +++++++++++-------- 8 files changed, 130 insertions(+), 37 deletions(-) diff --git a/browser/base/content/contentSearchUI.js b/browser/base/content/contentSearchUI.js index beee1d18a6f9..1394b6c1575a 100644 --- a/browser/base/content/contentSearchUI.js +++ b/browser/base/content/contentSearchUI.js @@ -33,6 +33,8 @@ this.ContentSearchUIController = (function() { * This will be sent with the search data for FHR to record the search. * @param searchPurpose * Sent with search data, see nsISearchEngine.getSubmission. + * @param isPrivateWindow + * Set to true if this instance is in a private window * @param idPrefix * The IDs of elements created by the object will be prefixed with this * string. @@ -42,12 +44,14 @@ this.ContentSearchUIController = (function() { tableParent, healthReportKey, searchPurpose, + isPrivateWindow, idPrefix = "" ) { this.input = inputElement; this._idPrefix = idPrefix; this._healthReportKey = healthReportKey; this._searchPurpose = searchPurpose; + this._isPrivateWindow = isPrivateWindow; let tableID = idPrefix + "searchSuggestionTable"; this.input.autocomplete = "off"; @@ -629,15 +633,21 @@ this.ContentSearchUIController = (function() { _onMsgState(state) { this.engines = state.engines; + + let currentEngine = state.currentEngine; + if (this._isPrivateWindow) { + currentEngine = state.currentPrivateEngine; + } + // No point updating the default engine (and the header) if there's no change. if ( this.defaultEngine && - this.defaultEngine.name == state.currentEngine.name && - this.defaultEngine.icon == state.currentEngine.icon + this.defaultEngine.name == currentEngine.name && + this.defaultEngine.icon == currentEngine.icon ) { return; } - this.defaultEngine = state.currentEngine; + this.defaultEngine = currentEngine; }, _onMsgCurrentState(state) { @@ -645,6 +655,17 @@ this.ContentSearchUIController = (function() { }, _onMsgCurrentEngine(engine) { + if (this._isPrivateWindow) { + return; + } + this.defaultEngine = engine; + this._pendingOneOffRefresh = true; + }, + + _onMsgCurrentPrivateEngine(engine) { + if (!this._isPrivateWindow) { + return; + } this.defaultEngine = engine; this._pendingOneOffRefresh = true; }, diff --git a/browser/components/about/AboutPrivateBrowsingHandler.jsm b/browser/components/about/AboutPrivateBrowsingHandler.jsm index 58d9370435b2..b1df640fbd3d 100644 --- a/browser/components/about/AboutPrivateBrowsingHandler.jsm +++ b/browser/components/about/AboutPrivateBrowsingHandler.jsm @@ -46,7 +46,8 @@ var AboutPrivateBrowsingHandler = { case "SearchHandoff": { let searchAlias = ""; let searchAliases = - Services.search.defaultEngine.wrappedJSObject.__internalAliases; + Services.search.defaultPrivateEngine.wrappedJSObject + .__internalAliases; if (searchAliases && searchAliases.length) { searchAlias = `${searchAliases[0]} `; } diff --git a/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js b/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js index ce34c666b78d..ec8df80fc2d9 100644 --- a/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js +++ b/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js @@ -80,6 +80,8 @@ document.addEventListener("DOMContentLoaded", function() { input, input.parentNode, "aboutprivatebrowsing", - "aboutprivatebrowsing" + "aboutprivatebrowsing", + // We know we're definitely in a PB window here. + true ); }); diff --git a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js index 4122d7a2ed68..fdc134535e9d 100644 --- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js +++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js @@ -36,6 +36,26 @@ async function testLinkOpensUrl({ win, tab, elementId, expectedUrl }) { ); } +let expectedEngineAlias; +let expectedIconURL; + +add_task(async function setup() { + await SpecialPowers.pushPrefEnv({ + set: [["browser.search.separatePrivateDefault", true]], + }); + + const originalPrivateDefault = await Services.search.getDefaultPrivate(); + // We have to use a built-in engine as we are currently hard-coding the aliases. + const privateEngine = await Services.search.getEngineByName("DuckDuckGo"); + await Services.search.setDefaultPrivate(privateEngine); + expectedEngineAlias = privateEngine.wrappedJSObject.__internalAliases[0]; + expectedIconURL = privateEngine.iconURI.spec; + + registerCleanupFunction(async () => { + await Services.search.setDefaultPrivate(originalPrivateDefault); + }); +}); + /** * Tests the private-browsing-myths link in "about:privatebrowsing". */ @@ -71,6 +91,23 @@ function urlBarHasNormalFocus(win) { ); } +/** + * Tests that we have the correct icon displayed. + */ +add_task(async function test_search_icon() { + let { win, tab } = await openAboutPrivateBrowsing(); + + await ContentTask.spawn(tab, expectedIconURL, async function(iconURL) { + is( + content.document.body.getAttribute("style"), + `--newtab-search-icon:url(${iconURL});`, + "Should have the correct icon URL for the logo" + ); + }); + + await BrowserTestUtils.closeWindow(win); +}); + /** * Tests the search hand-off on character keydown in "about:privatebrowsing". */ @@ -93,7 +130,7 @@ add_task(async function test_search_handoff_on_keydown() { ); }); ok(urlBarHasNormalFocus(win), "url bar has normal focused"); - is(win.gURLBar.value, "@google f", "url bar has search text"); + is(win.gURLBar.value, `${expectedEngineAlias} f`, "url bar has search text"); await UrlbarTestUtils.promiseSearchComplete(win); // Close the popup. await UrlbarTestUtils.promisePopupClose(win); @@ -151,7 +188,11 @@ add_task(async function test_search_handoff_on_paste() { await UrlbarTestUtils.promiseSearchComplete(win); ok(urlBarHasNormalFocus(win), "url bar has normal focused"); - is(win.gURLBar.value, "@google words", "url bar has search text"); + is( + win.gURLBar.value, + `${expectedEngineAlias} words`, + "url bar has search text" + ); await BrowserTestUtils.closeWindow(win); }); diff --git a/browser/components/search/test/browser/head.js b/browser/components/search/test/browser/head.js index ec9f3ec7093f..f6be5c464b40 100644 --- a/browser/components/search/test/browser/head.js +++ b/browser/components/search/test/browser/head.js @@ -67,6 +67,9 @@ function promiseEvent(aTarget, aEventName, aPreventDefault) { * - {String} [iconURL] The icon to use for the search engine. * - {Boolean} [setAsCurrent] Whether to set the new engine to be the * current engine or not. + * - {Boolean} [setAsCurrentPrivate] Whether to set the new engine to be the + * current private browsing mode engine or not. + * Defaults to false. * - {String} [testPath] Used to override the current test path if this * file is used from a different directory. * @returns {Promise} The promise is resolved once the engine is added, or @@ -78,20 +81,27 @@ async function promiseNewEngine(basename, options = {}) { options.setAsCurrent == undefined ? true : options.setAsCurrent; info("Waiting for engine to be added: " + basename); let url = getRootDirectory(options.testPath || gTestPath) + basename; - let current = await Services.search.getDefault(); let engine = await Services.search.addEngine( url, options.iconURL || "", false ); info("Search engine added: " + basename); + const current = await Services.search.getDefault(); if (setAsCurrent) { await Services.search.setDefault(engine); } + const currentPrivate = await Services.search.getDefaultPrivate(); + if (options.setAsCurrentPrivate) { + await Services.search.setDefaultPrivate(engine); + } registerCleanupFunction(async () => { if (setAsCurrent) { await Services.search.setDefault(current); } + if (options.setAsCurrentPrivate) { + await Services.search.setDefaultPrivate(currentPrivate); + } await Services.search.removeEngine(engine); info("Search engine removed: " + basename); }); diff --git a/browser/modules/ContentSearch.jsm b/browser/modules/ContentSearch.jsm index cd8e989a7a1e..57a5ab7c745a 100644 --- a/browser/modules/ContentSearch.jsm +++ b/browser/modules/ContentSearch.jsm @@ -366,7 +366,8 @@ var ContentSearch = { async currentStateObj() { let state = { engines: [], - currentEngine: await this._currentEngineObj(), + currentEngine: await this._currentEngineObj(false), + currentPrivateEngine: await this._currentEngineObj(true), }; let pref = Services.prefs.getCharPref("browser.search.hiddenOneOffs"); @@ -501,8 +502,11 @@ var ContentSearch = { async _onObserve(data) { if (data === "engine-default") { - let engine = await this._currentEngineObj(); + let engine = await this._currentEngineObj(false); this._broadcast("CurrentEngine", engine); + } else if (data === "engine-default-private") { + let engine = await this._currentEngineObj(true); + this._broadcast("CurrentPrivateEngine", engine); } else { let state = await this.currentStateObj(); this._broadcast("CurrentState", state); @@ -545,8 +549,9 @@ var ContentSearch = { ]; }, - async _currentEngineObj() { - let engine = Services.search.defaultEngine; + async _currentEngineObj(usePrivate) { + let engine = + Services.search[usePrivate ? "defaultPrivateEngine" : "defaultEngine"]; let favicon = engine.getIconURLBySize(16, 16); let placeholder = this._stringBundle.formatStringFromName( "searchWithEngine", diff --git a/browser/modules/test/browser/browser.ini b/browser/modules/test/browser/browser.ini index af7c6855dc9e..20024a030039 100644 --- a/browser/modules/test/browser/browser.ini +++ b/browser/modules/test/browser/browser.ini @@ -13,6 +13,7 @@ support-files = contentSearchSuggestions.xml !/browser/components/search/test/browser/head.js !/browser/components/search/test/browser/testEngine.xml + !/browser/components/search/test/browser/testEngine_diacritics.xml testEngine_chromeicon.xml [browser_EveryWindow.js] [browser_LiveBookmarkMigrator.js] diff --git a/browser/modules/test/browser/browser_ContentSearch.js b/browser/modules/test/browser/browser_ContentSearch.js index a76b4e0186b0..96a89f0fe03d 100644 --- a/browser/modules/test/browser/browser_ContentSearch.js +++ b/browser/modules/test/browser/browser_ContentSearch.js @@ -12,16 +12,18 @@ Services.scriptloader.loadSubScript( this ); -var originalEngine; - var arrayBufferIconTested = false; var plainURIIconTested = false; add_task(async function setup() { - originalEngine = await Services.search.getDefault(); + const originalEngine = await Services.search.getDefault(); + const originalPrivateEngine = await Services.search.getDefaultPrivate(); await SpecialPowers.pushPrefEnv({ - set: [["browser.newtab.preload", false]], + set: [ + ["browser.newtab.preload", false], + ["browser.search.separatePrivateDefault", true], + ], }); await promiseNewEngine("testEngine.xml", { @@ -30,12 +32,20 @@ add_task(async function setup() { "chrome://mochitests/content/browser/browser/components/search/test/browser/", }); + await promiseNewEngine("testEngine_diacritics.xml", { + setAsCurrent: false, + setAsCurrentPrivate: true, + testPath: + "chrome://mochitests/content/browser/browser/components/search/test/browser/", + }); + await promiseNewEngine("testEngine_chromeicon.xml", { setAsCurrent: false, }); registerCleanupFunction(async () => { await Services.search.setDefault(originalEngine); + await Services.search.setDefaultPrivate(originalPrivateEngine); }); }); @@ -56,22 +66,8 @@ add_task(async function GetState() { add_task(async function SetDefaultEngine() { let { mm } = await addTab(); - let newDefaultEngine = null; + let newDefaultEngine = await Services.search.getEngineByName("FooChromeIcon"); let oldDefaultEngine = await Services.search.getDefault(); - let engines = await Services.search.getVisibleEngines(); - for (let engine of engines) { - if (engine != oldDefaultEngine) { - newDefaultEngine = engine; - break; - } - } - if (!newDefaultEngine) { - info( - "Couldn't find a non-selected search engine, " + - "skipping this part of the test" - ); - return; - } mm.sendAsyncMessage(TEST_MSG, { type: "SetCurrentEngine", data: newDefaultEngine.name, @@ -91,14 +87,28 @@ add_task(async function SetDefaultEngine() { let msg = await searchPromise; checkMsg(msg, { type: "CurrentEngine", - data: await defaultEngineObj(newDefaultEngine), + data: await constructEngineObj(newDefaultEngine), }); await Services.search.setDefault(oldDefaultEngine); msg = await waitForTestMsg(mm, "CurrentEngine"); checkMsg(msg, { type: "CurrentEngine", - data: await defaultEngineObj(oldDefaultEngine), + data: await constructEngineObj(oldDefaultEngine), + }); +}); + +// ContentSearch.jsm doesn't support setting the private engine at this time +// as it doesn't need to, so we just test updating the default here. +add_task(async function setDefaultEnginePrivate() { + const engine = await Services.search.getEngineByName("FooChromeIcon"); + const { mm } = await addTab(); + let msgPromise = waitForTestMsg(mm, "CurrentPrivateEngine"); + await Services.search.setDefaultPrivate(engine); + let msg = await msgPromise; + checkMsg(msg, { + type: "CurrentPrivateEngine", + data: await constructEngineObj(engine), }); }); @@ -390,7 +400,10 @@ async function addTab() { var currentStateObj = async function() { let state = { engines: [], - currentEngine: await defaultEngineObj(), + currentEngine: await constructEngineObj(await Services.search.getDefault()), + currentPrivateEngine: await constructEngineObj( + await Services.search.getDefaultPrivate() + ), }; for (let engine of await Services.search.getVisibleEngines()) { let uri = engine.getIconURLBySize(16, 16); @@ -404,8 +417,7 @@ var currentStateObj = async function() { return state; }; -var defaultEngineObj = async function() { - let engine = await Services.search.getDefault(); +async function constructEngineObj(engine) { let uriFavicon = engine.getIconURLBySize(16, 16); let bundle = Services.strings.createBundle( "chrome://global/locale/autocomplete.properties" @@ -415,7 +427,7 @@ var defaultEngineObj = async function() { placeholder: bundle.formatStringFromName("searchWithEngine", [engine.name]), iconData: await iconDataFromURI(uriFavicon), }; -}; +} function iconDataFromURI(uri) { if (!uri) {