Bug 493051: avoid having addEngine select the engine by default, by adding an optional callback to let callers observe the successful addition of the engine, r=MattN

--HG--
extra : rebase_source : 1e67f4fbed4324e2d5b8d132cf07be60b3010cf9
This commit is contained in:
Gavin Sharp 2013-06-18 09:39:02 -04:00
parent 72d2cfc005
commit a53dcdb333
10 changed files with 232 additions and 46 deletions

View File

@ -488,17 +488,22 @@
<handlers>
<handler event="command"><![CDATA[
const target = event.originalTarget;
if (target.classList.contains("addengine-item")) {
if (target.engine) {
this.currentEngine = target.engine;
} else if (target.classList.contains("addengine-item")) {
var searchService =
Components.classes["@mozilla.org/browser/search-service;1"]
.getService(Components.interfaces.nsIBrowserSearchService);
// We only detect OpenSearch files
var type = Components.interfaces.nsISearchEngine.DATA_XML;
// Select the installed engine if the installation succeeds
var installCallback = {
onSuccess: engine => this.currentEngine = engine
}
searchService.addEngine(target.getAttribute("uri"), type,
target.getAttribute("src"), false);
target.getAttribute("src"), false,
installCallback);
}
else if (target.engine)
this.currentEngine = target.engine;
else
return;

View File

@ -30,8 +30,7 @@ function test() {
case "engine-added":
var engine = ss.getEngineByName("Bug 426329");
ok(engine, "Engine was added.");
//XXX Bug 493051
//ss.currentEngine = engine;
ss.currentEngine = engine;
break;
case "engine-current":
ok(ss.currentEngine.name == "Bug 426329", "currentEngine set");

View File

@ -16,8 +16,7 @@ function test() {
case "engine-added":
var engine = ss.getEngineByName(ENGINE_NAME);
ok(engine, "Engine was added.");
//XXX Bug 493051
//ss.currentEngine = engine;
ss.currentEngine = engine;
break;
case "engine-current":
is(ss.currentEngine.name, ENGINE_NAME, "currentEngine set");

View File

@ -42,20 +42,18 @@ function test() {
}
function addEngine(aCallback) {
function observer(aSub, aTopic, aData) {
switch (aData) {
case "engine-current":
ok(Services.search.currentEngine.name == "Bug 426329",
"currentEngine set");
aCallback();
break;
let installCallback = {
onSuccess: function (engine) {
Services.search.currentEngine = engine;
aCallback();
},
onError: function (errorCode) {
ok(false, "failed to install engine: " + errorCode);
}
}
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);
};
Services.search.addEngine(engineURL + "426329.xml",
Ci.nsISearchEngine.DATA_XML,
"data:image/x-icon,%00", false, installCallback);
}
function testOnWindow(aIsPrivate, aCallback) {

View File

@ -140,6 +140,30 @@ 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
*/
@ -186,8 +210,7 @@ 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; if no confirmation dialog is shown, the new engine will be
* used right away automatically.
* right away.
*
* @param engineURL
* The URL to the search engine's description file.
@ -207,11 +230,16 @@ 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);
in boolean confirm, [optional] in nsISearchInstallCallback callback);
/**
* Adds a new search engine, without asking the user for confirmation and

View File

@ -5,6 +5,7 @@
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");
@ -1150,7 +1151,10 @@ 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: true,
_useNow: false,
// A function to be invoked when this engine object's addition completes (or
// fails). Only used for installation via addEngine.
_installCallback: null,
// Where the engine was loaded from. Can be one of: SEARCH_APP_DIR,
// SEARCH_PROFILE_DIR, SEARCH_IN_EXTENSION.
__installLocation: null,
@ -1304,10 +1308,19 @@ Engine.prototype = {
*/
_onLoad: function SRCH_ENG_onLoad(aBytes, aEngine) {
/**
* Handle an error during the load of an engine by prompting the user to
* notify him that the load failed.
* Handle an error during the load of an engine by notifying the engine's
* error callback, if any.
*/
function onError(aErrorString, aTitleString) {
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);
if (aEngine._engineToUpdate) {
// We're in an update, so just fail quietly
LOG("updating " + aEngine._engineToUpdate.name + " failed");
@ -1317,8 +1330,8 @@ Engine.prototype = {
var brandName = brandBundle.GetStringFromName("brandShortName");
var searchBundle = Services.strings.createBundle(SEARCH_BUNDLE);
var msgStringName = aErrorString || "error_loading_engine_msg2";
var titleStringName = aTitleString || "error_loading_engine_title";
var msgStringName = strings.error || "error_loading_engine_msg2";
var titleStringName = strings.title || "error_loading_engine_title";
var title = searchBundle.GetStringFromName(titleStringName);
var text = searchBundle.formatStringFromName(msgStringName,
[brandName, aEngine._location],
@ -1328,7 +1341,7 @@ Engine.prototype = {
}
if (!aBytes) {
onError();
promptError();
return;
}
@ -1351,7 +1364,7 @@ Engine.prototype = {
aEngine._data = aBytes;
break;
default:
onError();
promptError();
LOG("_onLoad: Bogus engine _dataType: \"" + this._dataType + "\"");
return;
}
@ -1362,7 +1375,7 @@ Engine.prototype = {
} catch (ex) {
LOG("_onLoad: Failed to init engine!\n" + ex);
// Report an error to the user
onError();
promptError();
return;
}
@ -1371,9 +1384,9 @@ Engine.prototype = {
// otherwise we fail silently.
if (!engineToUpdate) {
if (Services.search.getEngineByName(aEngine.name)) {
if (aEngine._confirm)
onError("error_duplicate_engine_msg", "error_invalid_engine_title");
promptError({ error: "error_duplicate_engine_msg",
title: "error_invalid_engine_title"
}, Ci.nsISearchInstallCallback.ERROR_DUPLICATE_ENGINE);
LOG("_onLoad: duplicate engine found, bailing");
return;
}
@ -1386,8 +1399,10 @@ Engine.prototype = {
var confirmation = aEngine._confirmAddEngine();
LOG("_onLoad: confirm is " + confirmation.confirmed +
"; useNow is " + confirmation.useNow);
if (!confirmation.confirmed)
if (!confirmation.confirmed) {
onError();
return;
}
aEngine._useNow = confirmation.useNow;
}
@ -1414,6 +1429,7 @@ Engine.prototype = {
if (!newSelfURL || !newSelfURL._hasRelation("self")) {
LOG("_onLoad: updateURL missing in updated engine for " +
aEngine.name + " aborted");
onError();
return;
}
newUpdateURL = newSelfURL.template;
@ -1421,6 +1437,7 @@ Engine.prototype = {
if (oldUpdateURL != newUpdateURL) {
LOG("_onLoad: updateURLs do not match! Update of " + aEngine.name + " aborted");
onError();
return;
}
}
@ -1428,10 +1445,6 @@ 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
@ -1442,6 +1455,11 @@ 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();
}
},
/**
@ -3341,14 +3359,31 @@ SearchService.prototype = {
},
addEngine: function SRCH_SVC_addEngine(aEngineURL, aDataType, aIconURL,
aConfirm) {
aConfirm, aCallback) {
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);

View File

@ -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;

View File

@ -0,0 +1,123 @@
/* 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");
// Override the prompt service and nsIPrompt, since the search service currently
// prompts in response to certain installation failures we test here
// XXX this should disappear once bug 863474 is fixed
function replaceService(contractID, component) {
let registrar = Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
let cid = registrar.contractIDToCID(contractID);
let oldFactory = Components.manager.getClassObject(Components.classes[contractID],
Ci.nsIFactory);
registrar.unregisterFactory(cid, oldFactory);
let factory = {
createInstance: function(aOuter, aIid) {
if (aOuter != null)
throw Components.results.NS_ERROR_NO_AGGREGATION;
return component.QueryInterface(aIid);
}
};
registrar.registerFactory(cid, "", contractID, factory);
}
// Only need to stub the methods actually called by nsSearchService
let promptService = {
QueryInterface: XPCOMUtils.generateQI([Ci.nsIPromptService]),
confirmEx: function() {}
};
let prompt = {
QueryInterface: XPCOMUtils.generateQI([Ci.nsIPrompt]),
alert: function() {}
};
replaceService("@mozilla.org/embedcomp/prompt-service;1", promptService);
replaceService("@mozilla.org/prompter;1", prompt);
// 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();
}

View File

@ -40,8 +40,7 @@ function search_observer(subject, topic, data) {
let retrievedEngine = Services.search.getEngineByName("Test search engine");
do_check_eq(engine, retrievedEngine);
Services.search.defaultEngine = engine;
// XXX bug 493051
// Services.search.currentEngine = engine;
Services.search.currentEngine = engine;
do_execute_soon(function () {
Services.search.removeEngine(engine);
});

View File

@ -19,4 +19,4 @@ skip-if = debug && os == "linux"
[test_defaultEngine.js]
[test_prefSync.js]
[test_notifications.js]
[test_addEngine_callback.js]