Bug 1408472 - Read-only browserSettings should return false on write attempts, r?mixedpuppy
This updates browserSettings so that calling set and clear return false, rather than undefined,
for read-only settings. It also returns "not_controllable" rather than "controllable_by_this_extension"
for levelOfControl from calls to get for read-only settings.
MozReview-Commit-ID: 9c0bKDaYIC7
--- a/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js
+++ b/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js
@@ -8,19 +8,19 @@ XPCOMUtils.defineLazyModuleGetter(this,
XPCOMUtils.defineLazyModuleGetter(this, "Services",
"resource://gre/modules/Services.jsm");
// Named this way so they correspond to the extensions
const HOME_URI_2 = "http://example.com/";
const HOME_URI_3 = "http://example.org/";
const HOME_URI_4 = "http://example.net/";
-const CONTROLLABLE = "controllable_by_this_extension";
const CONTROLLED_BY_THIS = "controlled_by_this_extension";
const CONTROLLED_BY_OTHER = "controlled_by_other_extensions";
+const NOT_CONTROLLABLE = "not_controllable";
const HOMEPAGE_URL_PREF = "browser.startup.homepage";
const getHomePageURL = () => {
return Services.prefs.getComplexValue(
HOMEPAGE_URL_PREF, Ci.nsIPrefLocalizedString).data;
};
@@ -30,21 +30,23 @@ add_task(async function test_multiple_ex
function background() {
browser.test.onMessage.addListener(async msg => {
switch (msg) {
case "checkHomepage":
let homepage = await browser.browserSettings.homepageOverride.get({});
browser.test.sendMessage("homepage", homepage);
break;
case "trySet":
- await browser.browserSettings.homepageOverride.set({value: "foo"});
+ let setResult = await browser.browserSettings.homepageOverride.set({value: "foo"});
+ browser.test.assertFalse(setResult, "Calling homepageOverride.set returns false.");
browser.test.sendMessage("homepageSet");
break;
case "tryClear":
- await browser.browserSettings.homepageOverride.clear({});
+ let clearResult = await browser.browserSettings.homepageOverride.clear({});
+ browser.test.assertFalse(clearResult, "Calling homepageOverride.clear returns false.");
browser.test.sendMessage("homepageCleared");
break;
}
});
}
let extObj = {
manifest: {
@@ -77,17 +79,17 @@ add_task(async function test_multiple_ex
is(homepage.levelOfControl, expectedLevelOfControl,
`homepageOverride setting returns the expected levelOfControl: ${expectedLevelOfControl}.`);
}
await ext1.startup();
is(getHomePageURL(), defaultHomePage,
"Home url should be the default");
- await checkHomepageOverride(ext1, getHomePageURL(), CONTROLLABLE);
+ await checkHomepageOverride(ext1, getHomePageURL(), NOT_CONTROLLABLE);
// Because we are expecting the pref to change when we start or unload, we
// need to wait on a pref change. This is because the pref management is
// async and can happen after the startup/unload is finished.
let prefPromise = promisePrefChangeObserved(HOMEPAGE_URL_PREF);
await ext2.startup();
await prefPromise;
@@ -151,17 +153,17 @@ add_task(async function test_multiple_ex
prefPromise = promisePrefChangeObserved(HOMEPAGE_URL_PREF);
await ext3.unload();
await prefPromise;
is(getHomePageURL(), defaultHomePage,
"Home url should be reset to default");
await ext5.startup();
- await checkHomepageOverride(ext5, defaultHomePage, CONTROLLABLE);
+ await checkHomepageOverride(ext5, defaultHomePage, NOT_CONTROLLABLE);
await ext5.unload();
});
const HOME_URI_1 = "http://example.com/";
const USER_URI = "http://example.edu/";
add_task(async function test_extension_setting_home_page_back() {
let defaultHomePage = getHomePageURL();
--- a/browser/components/extensions/test/xpcshell/test_ext_url_overrides_newtab.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_url_overrides_newtab.js
@@ -39,33 +39,35 @@ function awaitEvent(eventName) {
const DEFAULT_NEW_TAB_URL = aboutNewTabService.newTabURL;
add_task(async function test_multiple_extensions_overriding_newtab_page() {
const NEWTAB_URI_2 = "webext-newtab-1.html";
const NEWTAB_URI_3 = "webext-newtab-2.html";
const EXT_2_ID = "ext2@tests.mozilla.org";
const EXT_3_ID = "ext3@tests.mozilla.org";
- const CONTROLLABLE = "controllable_by_this_extension";
const CONTROLLED_BY_THIS = "controlled_by_this_extension";
const CONTROLLED_BY_OTHER = "controlled_by_other_extensions";
+ const NOT_CONTROLLABLE = "not_controllable";
function background() {
browser.test.onMessage.addListener(async msg => {
switch (msg) {
case "checkNewTabPage":
let newTabPage = await browser.browserSettings.newTabPageOverride.get({});
browser.test.sendMessage("newTabPage", newTabPage);
break;
case "trySet":
- await browser.browserSettings.newTabPageOverride.set({value: "foo"});
+ let setResult = await browser.browserSettings.newTabPageOverride.set({value: "foo"});
+ browser.test.assertFalse(setResult, "Calling newTabPageOverride.set returns false.");
browser.test.sendMessage("newTabPageSet");
break;
case "tryClear":
- await browser.browserSettings.newTabPageOverride.clear({});
+ let clearResult = await browser.browserSettings.newTabPageOverride.clear({});
+ browser.test.assertFalse(clearResult, "Calling newTabPageOverride.clear returns false.");
browser.test.sendMessage("newTabPageCleared");
break;
}
});
}
async function checkNewTabPageOverride(ext, expectedValue, expectedLevelOfControl) {
ext.sendMessage("checkNewTabPage");
@@ -100,17 +102,17 @@ add_task(async function test_multiple_ex
"newTabURL is set to the default.");
await promiseStartupManager();
await ext1.startup();
equal(aboutNewTabService.newTabURL, DEFAULT_NEW_TAB_URL,
"newTabURL is still set to the default.");
- await checkNewTabPageOverride(ext1, aboutNewTabService.newTabURL, CONTROLLABLE);
+ await checkNewTabPageOverride(ext1, aboutNewTabService.newTabURL, NOT_CONTROLLABLE);
await ext2.startup();
ok(aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_2),
"newTabURL is overriden by the second extension.");
await checkNewTabPageOverride(ext1, NEWTAB_URI_2, CONTROLLED_BY_OTHER);
// Verify that calling set and clear do nothing.
ext2.sendMessage("trySet");
@@ -123,17 +125,17 @@ add_task(async function test_multiple_ex
// Disable the second extension.
let addon = await AddonManager.getAddonByID(EXT_2_ID);
let disabledPromise = awaitEvent("shutdown");
addon.userDisabled = true;
await disabledPromise;
equal(aboutNewTabService.newTabURL, DEFAULT_NEW_TAB_URL,
"newTabURL url is reset to the default after second extension is disabled.");
- await checkNewTabPageOverride(ext1, aboutNewTabService.newTabURL, CONTROLLABLE);
+ await checkNewTabPageOverride(ext1, aboutNewTabService.newTabURL, NOT_CONTROLLABLE);
// Re-enable the second extension.
let enabledPromise = awaitEvent("ready");
addon.userDisabled = false;
await enabledPromise;
ok(aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_2),
"newTabURL is overriden by the second extension.");
await checkNewTabPageOverride(ext2, NEWTAB_URI_2, CONTROLLED_BY_THIS);
--- a/toolkit/components/extensions/ext-browserSettings.js
+++ b/toolkit/components/extensions/ext-browserSettings.js
@@ -22,34 +22,41 @@ const HOMEPAGE_URL_PREF = "browser.start
const URL_STORE_TYPE = "url_overrides";
const NEW_TAB_OVERRIDE_SETTING = "newTabURL";
const PERM_DENY_ACTION = Services.perms.DENY_ACTION;
const getSettingsAPI = (extension, name, callback, storeType, readOnly = false) => {
return {
async get(details) {
- return {
- levelOfControl: details.incognito ?
+ let levelOfControl = details.incognito ?
+ "not_controllable" :
+ await ExtensionPreferencesManager.getLevelOfControl(
+ extension.id, name, storeType);
+ levelOfControl =
+ (readOnly && levelOfControl === "controllable_by_this_extension") ?
"not_controllable" :
- await ExtensionPreferencesManager.getLevelOfControl(
- extension.id, name, storeType),
+ levelOfControl;
+ return {
+ levelOfControl,
value: await callback(),
};
},
set(details) {
if (!readOnly) {
return ExtensionPreferencesManager.setSetting(
extension.id, name, details.value);
}
+ return false;
},
clear(details) {
if (!readOnly) {
return ExtensionPreferencesManager.removeSetting(extension.id, name);
}
+ return false;
},
};
};
// Add settings objects for supported APIs to the preferences manager.
ExtensionPreferencesManager.addSetting("allowPopupsForUserEvents", {
prefNames: [
"dom.popup_allowed_events",