diff --git a/browser/components/search/content/search.xml b/browser/components/search/content/search.xml index 0a1dcaf18384..e105f527f8df 100644 --- a/browser/components/search/content/search.xml +++ b/browser/components/search/content/search.xml @@ -488,22 +488,17 @@ this.currentEngine = engine - } searchService.addEngine(target.getAttribute("uri"), type, - target.getAttribute("src"), false, - installCallback); + target.getAttribute("src"), false); } + else if (target.engine) + this.currentEngine = target.engine; else return; diff --git a/browser/components/search/test/browser_426329.js b/browser/components/search/test/browser_426329.js index 4b2310cd5a3b..585da8e6bc4d 100644 --- a/browser/components/search/test/browser_426329.js +++ b/browser/components/search/test/browser_426329.js @@ -30,7 +30,8 @@ function test() { case "engine-added": var engine = ss.getEngineByName("Bug 426329"); ok(engine, "Engine was added."); - ss.currentEngine = engine; + //XXX Bug 493051 + //ss.currentEngine = engine; break; case "engine-current": ok(ss.currentEngine.name == "Bug 426329", "currentEngine set"); diff --git a/browser/components/search/test/browser_contextmenu.js b/browser/components/search/test/browser_contextmenu.js index f34bbc60bb6e..3ac69dbbaa3c 100644 --- a/browser/components/search/test/browser_contextmenu.js +++ b/browser/components/search/test/browser_contextmenu.js @@ -16,7 +16,8 @@ function test() { case "engine-added": var engine = ss.getEngineByName(ENGINE_NAME); ok(engine, "Engine was added."); - ss.currentEngine = engine; + //XXX Bug 493051 + //ss.currentEngine = engine; break; case "engine-current": is(ss.currentEngine.name, ENGINE_NAME, "currentEngine set"); diff --git a/browser/components/search/test/browser_private_search_perwindowpb.js b/browser/components/search/test/browser_private_search_perwindowpb.js index 24c733e5e615..2dc911bd3ace 100644 --- a/browser/components/search/test/browser_private_search_perwindowpb.js +++ b/browser/components/search/test/browser_private_search_perwindowpb.js @@ -42,18 +42,20 @@ function test() { } function addEngine(aCallback) { - let installCallback = { - onSuccess: function (engine) { - Services.search.currentEngine = engine; - aCallback(); - }, - onError: function (errorCode) { - ok(false, "failed to install engine: " + errorCode); + function observer(aSub, aTopic, aData) { + switch (aData) { + case "engine-current": + ok(Services.search.currentEngine.name == "Bug 426329", + "currentEngine set"); + aCallback(); + break; } - }; - Services.search.addEngine(engineURL + "426329.xml", - Ci.nsISearchEngine.DATA_XML, - "data:image/x-icon,%00", false, installCallback); + } + + Services.obs.addObserver(observer, "browser-search-engine-modified", false); + Services.search.addEngine( + engineURL + "426329.xml", Ci.nsISearchEngine.DATA_XML, + "data:image/x-icon,%00", false); } function testOnWindow(aIsPrivate, aCallback) { diff --git a/netwerk/base/public/nsIBrowserSearchService.idl b/netwerk/base/public/nsIBrowserSearchService.idl index 726a64a0333d..b441050a72df 100644 --- a/netwerk/base/public/nsIBrowserSearchService.idl +++ b/netwerk/base/public/nsIBrowserSearchService.idl @@ -140,30 +140,6 @@ interface nsISearchEngine : nsISupports }; -[scriptable, uuid(9fc39136-f08b-46d3-b232-96f4b7b0e235)] -interface nsISearchInstallCallback : nsISupports -{ - const unsigned long ERROR_UNKNOWN_FAILURE = 0x1; - const unsigned long ERROR_DUPLICATE_ENGINE = 0x2; - - /** - * Called to indicate that the engine addition process succeeded. - * - * @param engine - * The nsISearchEngine object that was added (will not be null). - */ - void onSuccess(in nsISearchEngine engine); - - /** - * Called to indicate that the engine addition process failed. - * - * @param errorCode - * One of the ERROR_* values described above indicating the cause of - * the failure. - */ - void onError(in unsigned long errorCode); -}; - /** * Callback for asynchronous initialization of nsIBrowserSearchService */ @@ -210,7 +186,8 @@ interface nsIBrowserSearchService : nsISupports * Adds a new search engine from the file at the supplied URI, optionally * asking the user for confirmation first. If a confirmation dialog is * shown, it will offer the option to begin using the newly added engine - * right away. + * right away; if no confirmation dialog is shown, the new engine will be + * used right away automatically. * * @param engineURL * The URL to the search engine's description file. @@ -230,16 +207,11 @@ interface nsIBrowserSearchService : nsISupports * value is false, the engine will be added to the list upon successful * load, but it will not be selected as the current engine. * - * @param callback - * A nsISearchInstallCallback that will be notified when the - * addition is complete, or if the addition fails. It will not be - * called if addEngine throws an exception. - * * @throws NS_ERROR_FAILURE if the type is invalid, or if the description * file cannot be successfully loaded. */ void addEngine(in AString engineURL, in long dataType, in AString iconURL, - in boolean confirm, [optional] in nsISearchInstallCallback callback); + in boolean confirm); /** * Adds a new search engine, without asking the user for confirmation and diff --git a/toolkit/components/search/nsSearchService.js b/toolkit/components/search/nsSearchService.js index 8a154c15a86e..27436ea1e077 100644 --- a/toolkit/components/search/nsSearchService.js +++ b/toolkit/components/search/nsSearchService.js @@ -5,7 +5,6 @@ const Ci = Components.interfaces; const Cc = Components.classes; const Cr = Components.results; -const Cu = Components.utils; Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); Components.utils.import("resource://gre/modules/Services.jsm"); @@ -1151,10 +1150,7 @@ Engine.prototype = { _confirm: false, // Whether to set this as the current engine as soon as it is loaded. This // is only used when the engine is first added to the list. - _useNow: false, - // A function to be invoked when this engine object's addition completes (or - // fails). Only used for installation via addEngine. - _installCallback: null, + _useNow: true, // Where the engine was loaded from. Can be one of: SEARCH_APP_DIR, // SEARCH_PROFILE_DIR, SEARCH_IN_EXTENSION. __installLocation: null, @@ -1308,19 +1304,10 @@ Engine.prototype = { */ _onLoad: function SRCH_ENG_onLoad(aBytes, aEngine) { /** - * Handle an error during the load of an engine by notifying the engine's - * error callback, if any. + * Handle an error during the load of an engine by prompting the user to + * notify him that the load failed. */ - function onError(errorCode = Ci.nsISearchInstallCallback.ERROR_UNKNOWN_FAILURE) { - // Notify the callback of the failure - if (aEngine._installCallback) { - aEngine._installCallback(errorCode); - } - } - - function promptError(strings = {}, error = undefined) { - onError(error); - + function onError(aErrorString, aTitleString) { if (aEngine._engineToUpdate) { // We're in an update, so just fail quietly LOG("updating " + aEngine._engineToUpdate.name + " failed"); @@ -1330,8 +1317,8 @@ Engine.prototype = { var brandName = brandBundle.GetStringFromName("brandShortName"); var searchBundle = Services.strings.createBundle(SEARCH_BUNDLE); - var msgStringName = strings.error || "error_loading_engine_msg2"; - var titleStringName = strings.title || "error_loading_engine_title"; + var msgStringName = aErrorString || "error_loading_engine_msg2"; + var titleStringName = aTitleString || "error_loading_engine_title"; var title = searchBundle.GetStringFromName(titleStringName); var text = searchBundle.formatStringFromName(msgStringName, [brandName, aEngine._location], @@ -1341,7 +1328,7 @@ Engine.prototype = { } if (!aBytes) { - promptError(); + onError(); return; } @@ -1364,7 +1351,7 @@ Engine.prototype = { aEngine._data = aBytes; break; default: - promptError(); + onError(); LOG("_onLoad: Bogus engine _dataType: \"" + this._dataType + "\""); return; } @@ -1375,7 +1362,7 @@ Engine.prototype = { } catch (ex) { LOG("_onLoad: Failed to init engine!\n" + ex); // Report an error to the user - promptError(); + onError(); return; } @@ -1384,9 +1371,9 @@ Engine.prototype = { // otherwise we fail silently. if (!engineToUpdate) { if (Services.search.getEngineByName(aEngine.name)) { - promptError({ error: "error_duplicate_engine_msg", - title: "error_invalid_engine_title" - }, Ci.nsISearchInstallCallback.ERROR_DUPLICATE_ENGINE); + if (aEngine._confirm) + onError("error_duplicate_engine_msg", "error_invalid_engine_title"); + LOG("_onLoad: duplicate engine found, bailing"); return; } @@ -1399,10 +1386,8 @@ Engine.prototype = { var confirmation = aEngine._confirmAddEngine(); LOG("_onLoad: confirm is " + confirmation.confirmed + "; useNow is " + confirmation.useNow); - if (!confirmation.confirmed) { - onError(); + if (!confirmation.confirmed) return; - } aEngine._useNow = confirmation.useNow; } @@ -1429,7 +1414,6 @@ Engine.prototype = { if (!newSelfURL || !newSelfURL._hasRelation("self")) { LOG("_onLoad: updateURL missing in updated engine for " + aEngine.name + " aborted"); - onError(); return; } newUpdateURL = newSelfURL.template; @@ -1437,7 +1421,6 @@ Engine.prototype = { if (oldUpdateURL != newUpdateURL) { LOG("_onLoad: updateURLs do not match! Update of " + aEngine.name + " aborted"); - onError(); return; } } @@ -1445,6 +1428,10 @@ Engine.prototype = { // Set the new engine's icon, if it doesn't yet have one. if (!aEngine._iconURI && engineToUpdate._iconURI) aEngine._iconURI = engineToUpdate._iconURI; + + // Clear the "use now" flag since we don't want to be changing the + // current engine for an update. + aEngine._useNow = false; } // Write the engine to file. For readOnly engines, they'll be stored in the @@ -1455,11 +1442,6 @@ Engine.prototype = { // Notify the search service of the successful load. It will deal with // updates by checking aEngine._engineToUpdate. notifyAction(aEngine, SEARCH_ENGINE_LOADED); - - // Notify the callback if needed - if (aEngine._installCallback) { - aEngine._installCallback(); - } }, /** @@ -3359,31 +3341,14 @@ SearchService.prototype = { }, addEngine: function SRCH_SVC_addEngine(aEngineURL, aDataType, aIconURL, - aConfirm, aCallback) { + aConfirm) { LOG("addEngine: Adding \"" + aEngineURL + "\"."); this._ensureInitialized(); try { var uri = makeURI(aEngineURL); var engine = new Engine(uri, aDataType, false); - if (aCallback) { - engine._installCallback = function (errorCode) { - try { - if (errorCode == null) - aCallback.onSuccess(engine); - else - aCallback.onError(errorCode); - } catch (ex) { - Cu.reportError("Error invoking addEngine install callback: " + ex); - } - // Clear the reference to the callback now that it's been invoked. - engine._installCallback = null; - }; - } engine._initFromURI(); } catch (ex) { - // Drop the reference to the callback, if set - if (engine) - engine._installCallback = null; FAIL("addEngine: Error adding engine:\n" + ex, Cr.NS_ERROR_FAILURE); } engine._setIcon(aIconURL, false); diff --git a/toolkit/components/search/tests/xpcshell/head_search.js b/toolkit/components/search/tests/xpcshell/head_search.js index 4eb221642fa7..4775ae4a0177 100644 --- a/toolkit/components/search/tests/xpcshell/head_search.js +++ b/toolkit/components/search/tests/xpcshell/head_search.js @@ -97,7 +97,7 @@ function afterCache(callback) Services.obs.addObserver(obs, "browser-search-service", false); } -function parseJsonFromStream(aInputStream) { +function parseJsonFromStream(aInputStream) { const json = Cc["@mozilla.org/dom/json;1"].createInstance(Components.interfaces.nsIJSON); const data = json.decodeFromStream(aInputStream, aInputStream.available()); return data; diff --git a/toolkit/components/search/tests/xpcshell/test_addEngine_callback.js b/toolkit/components/search/tests/xpcshell/test_addEngine_callback.js deleted file mode 100644 index 7e5446f7b42b..000000000000 --- a/toolkit/components/search/tests/xpcshell/test_addEngine_callback.js +++ /dev/null @@ -1,89 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ */ - -/* - * Tests covering nsIBrowserSearchService::addEngine's optional callback. - */ - -"use strict"; - -const Ci = Components.interfaces; - -Components.utils.import("resource://testing-common/httpd.js"); - -// First test inits the search service -add_test(function init_search_service() { - Services.search.init(function (status) { - if (!Components.isSuccessCode(status)) - do_throw("Failed to initialize search service"); - - run_next_test(); - }); -}); - -// Simple test of the search callback -add_test(function simple_callback_test() { - let searchCallback = { - onSuccess: function (engine) { - do_check_true(!!engine); - do_check_neq(engine.name, Services.search.defaultEngine.name); - run_next_test(); - }, - onError: function (errorCode) { - do_throw("search callback returned error: " + errorCode); - } - } - Services.search.addEngine("http://localhost:4444/data/engine.xml", - Ci.nsISearchEngine.DATA_XML, - null, false, searchCallback); -}); - -// Test of the search callback on duplicate engine failures -add_test(function duplicate_failure_test() { - let searchCallback = { - onSuccess: function (engine) { - do_throw("this addition should not have succeeded"); - }, - onError: function (errorCode) { - do_check_true(!!errorCode); - do_check_eq(errorCode, Ci.nsISearchInstallCallback.ERROR_DUPLICATE_ENGINE); - run_next_test(); - } - } - // Re-add the same engine added in the previous test - Services.search.addEngine("http://localhost:4444/data/engine.xml", - Ci.nsISearchEngine.DATA_XML, - null, false, searchCallback); -}); - -// Test of the search callback on failure to load the engine failures -add_test(function load_failure_test() { - let searchCallback = { - onSuccess: function (engine) { - do_throw("this addition should not have succeeded"); - }, - onError: function (errorCode) { - do_check_true(!!errorCode); - do_check_eq(errorCode, Ci.nsISearchInstallCallback.ERROR_UNKNOWN_FAILURE); - run_next_test(); - } - } - // Try adding an engine that doesn't exist - Services.search.addEngine("http://invalid/data/engine.xml", - Ci.nsISearchEngine.DATA_XML, - null, false, searchCallback); -}); - -function run_test() { - updateAppInfo(); - - let httpServer = new HttpServer(); - httpServer.start(4444); - httpServer.registerDirectory("/", do_get_cwd()); - - do_register_cleanup(function cleanup() { - httpServer.stop(function() {}); - }); - - run_next_test(); -} diff --git a/toolkit/components/search/tests/xpcshell/test_notifications.js b/toolkit/components/search/tests/xpcshell/test_notifications.js index 485ed0e6e136..a75a77d60671 100644 --- a/toolkit/components/search/tests/xpcshell/test_notifications.js +++ b/toolkit/components/search/tests/xpcshell/test_notifications.js @@ -40,7 +40,8 @@ function search_observer(subject, topic, data) { let retrievedEngine = Services.search.getEngineByName("Test search engine"); do_check_eq(engine, retrievedEngine); Services.search.defaultEngine = engine; - Services.search.currentEngine = engine; + // XXX bug 493051 + // Services.search.currentEngine = engine; do_execute_soon(function () { Services.search.removeEngine(engine); }); diff --git a/toolkit/components/search/tests/xpcshell/xpcshell.ini b/toolkit/components/search/tests/xpcshell/xpcshell.ini index 282d2ba9a068..c76336be658b 100644 --- a/toolkit/components/search/tests/xpcshell/xpcshell.ini +++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini @@ -19,4 +19,4 @@ skip-if = debug && os == "linux" [test_defaultEngine.js] [test_prefSync.js] [test_notifications.js] -[test_addEngine_callback.js] +