From 2de58add2240394d84ff8ff32b78d71370066590 Mon Sep 17 00:00:00 2001 From: Dale Harvey Date: Fri, 3 Aug 2018 16:00:55 +0100 Subject: [PATCH] Bug 1477273 - Allow autoplay-media permission to be temporarily allowed. r=johannh MozReview-Commit-ID: JlnH2f1KW3U --HG-- extra : rebase_source : 13fb179fd85a47f1842b7715e82e92a30c4c4784 --- browser/modules/PermissionUI.jsm | 6 ++- browser/modules/SitePermissions.jsm | 54 ++++++++++++------- .../test/browser/browser_SitePermissions.js | 23 ++++++++ 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/browser/modules/PermissionUI.jsm b/browser/modules/PermissionUI.jsm index ba38c6f75230..faa94d38564c 100644 --- a/browser/modules/PermissionUI.jsm +++ b/browser/modules/PermissionUI.jsm @@ -337,8 +337,10 @@ var PermissionPromptPrototype = { this.permissionKey, promptAction.action, scope); - } else if (promptAction.action == SitePermissions.BLOCK) { - // Temporarily store BLOCK permissions only. + } else if (promptAction.action == SitePermissions.BLOCK || + SitePermissions.permitTemporaryAllow(this.permissionKey)) { + // Temporarily store BLOCK permissions only unless permission object + // sets permitTemporaryAllow: true // SitePermissions does not consider subframes when storing temporary // permissions on a tab, thus storing ALLOW could be exploited. SitePermissions.set(this.principal.URI, diff --git a/browser/modules/SitePermissions.jsm b/browser/modules/SitePermissions.jsm index 08c8f3a4d9b9..57dbb876a454 100644 --- a/browser/modules/SitePermissions.jsm +++ b/browser/modules/SitePermissions.jsm @@ -11,7 +11,7 @@ var gStringBundle = Services.strings.createBundle("chrome://browser/locale/sitePermissions.properties"); /** - * A helper module to manage temporarily blocked permissions. + * A helper module to manage temporary permissions. * * Permissions are keyed by browser, so methods take a Browser * element to identify the corresponding permission set. @@ -20,7 +20,7 @@ var gStringBundle = * automatically cleared once the browser stops existing * (once there are no other references to the browser object); */ -const TemporaryBlockedPermissions = { +const TemporaryPermissions = { // This is a three level deep map with the following structure: // // Browser => { @@ -38,20 +38,20 @@ const TemporaryBlockedPermissions = { // Private helper method that bundles some shared behavior for // get() and getAll(), e.g. deleting permissions when they have expired. - _get(entry, prePath, id, timeStamp) { - if (timeStamp == null) { + _get(entry, prePath, id, permission) { + if (permission == null || permission.timeStamp == null) { delete entry[prePath][id]; return null; } - if (timeStamp + SitePermissions.temporaryPermissionExpireTime < Date.now()) { + if (permission.timeStamp + SitePermissions.temporaryPermissionExpireTime < Date.now()) { delete entry[prePath][id]; return null; } - return {id, state: SitePermissions.BLOCK, scope: SitePermissions.SCOPE_TEMPORARY}; + return {id, state: permission.state, scope: SitePermissions.SCOPE_TEMPORARY}; }, // Sets a new permission for the specified browser. - set(browser, id) { + set(browser, id, state) { if (!browser) { return; } @@ -63,7 +63,7 @@ const TemporaryBlockedPermissions = { if (!entry[prePath]) { entry[prePath] = {}; } - entry[prePath][id] = Date.now(); + entry[prePath][id] = {timeStamp: Date.now(), state}; }, // Removes a permission with the specified id for the specified browser. @@ -292,7 +292,7 @@ var SitePermissions = { getAllForBrowser(browser) { let permissions = {}; - for (let permission of TemporaryBlockedPermissions.getAll(browser)) { + for (let permission of TemporaryPermissions.getAll(browser)) { permission.scope = this.SCOPE_TEMPORARY; permissions[permission.id] = permission; } @@ -417,6 +417,23 @@ var SitePermissions = { return false; }, + /* + * Return whether SitePermissions is permitted to store a TEMPORARY ALLOW + * state for a particular permission. + * + * @param {string} permissionID + * The ID to get the state for. + * + * @return boolean Whether storing TEMPORARY ALLOW is permitted. + */ + permitTemporaryAllow(permissionID) { + if (permissionID in gPermissionObject && + gPermissionObject[permissionID].permitTemporaryAllow) + return gPermissionObject[permissionID].permitTemporaryAllow; + + return false; + }, + /** * Returns the state and scope of a particular permission for a given URI. * @@ -461,7 +478,7 @@ var SitePermissions = { if (result.state == defaultState) { // If there's no persistent permission saved, check if we have something // set temporarily. - let value = TemporaryBlockedPermissions.get(browser, permissionID); + let value = TemporaryPermissions.get(browser, permissionID); if (value) { result.state = value.state; @@ -520,9 +537,9 @@ var SitePermissions = { // URI and only consider the current browser URI, to avoid notification spamming. // // If you ever consider removing this line, you likely want to implement - // a more fine-grained TemporaryBlockedPermissions that temporarily blocks for the + // a more fine-grained TemporaryPermissions that temporarily blocks for the // entire browser, but temporarily allows only for specific frames. - if (state != this.BLOCK) { + if (state != this.BLOCK && !this.permitTemporaryAllow(permissionID)) { throw "'Block' is the only permission we can save temporarily on a browser"; } @@ -530,7 +547,7 @@ var SitePermissions = { throw "TEMPORARY scoped permissions require a browser object"; } - TemporaryBlockedPermissions.set(browser, permissionID); + TemporaryPermissions.set(browser, permissionID, state); browser.dispatchEvent(new browser.ownerGlobal .CustomEvent("PermissionStateChange")); @@ -562,10 +579,10 @@ var SitePermissions = { if (this.isSupportedURI(uri)) Services.perms.remove(uri, permissionID); - // TemporaryBlockedPermissions.get() deletes expired permissions automatically, - if (TemporaryBlockedPermissions.get(browser, permissionID)) { + // TemporaryPermissions.get() deletes expired permissions automatically, + if (TemporaryPermissions.get(browser, permissionID)) { // If it exists but has not expired, remove it explicitly. - TemporaryBlockedPermissions.remove(browser, permissionID); + TemporaryPermissions.remove(browser, permissionID); // Send a PermissionStateChange event only if the permission hasn't expired. browser.dispatchEvent(new browser.ownerGlobal .CustomEvent("PermissionStateChange")); @@ -579,7 +596,7 @@ var SitePermissions = { * The browser object to clear. */ clearTemporaryPermissions(browser) { - TemporaryBlockedPermissions.clear(browser); + TemporaryPermissions.clear(browser); }, /** @@ -592,7 +609,7 @@ var SitePermissions = { * The browser object to copy to. */ copyTemporaryPermissions(browser, newBrowser) { - TemporaryBlockedPermissions.copy(browser, newBrowser); + TemporaryPermissions.copy(browser, newBrowser); }, /** @@ -702,6 +719,7 @@ var gPermissionObject = { "autoplay-media": { exactHostMatch: true, showGloballyBlocked: true, + permitTemporaryAllow: true, getDefault() { let state = Services.prefs.getIntPref("media.autoplay.default", Ci.nsIAutoplay.PROMPT); diff --git a/browser/modules/test/browser/browser_SitePermissions.js b/browser/modules/test/browser/browser_SitePermissions.js index 94121c622682..ef3a306e2a85 100644 --- a/browser/modules/test/browser/browser_SitePermissions.js +++ b/browser/modules/test/browser/browser_SitePermissions.js @@ -19,6 +19,29 @@ add_task(async function testTempAllowThrows() { }); }); +// Tests that we can set TEMPORARY ALLOW permissions for autoplay-media +add_task(async function testTempAutoplayAllowed() { + let uri = Services.io.newURI("https://example.com"); + let permId = "autoplay-media"; + + let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, uri.spec); + + SitePermissions.set(uri, permId, SitePermissions.ALLOW, + SitePermissions.SCOPE_TEMPORARY, tab.linkedBrowser); + + let permissions = SitePermissions.getAllPermissionDetailsForBrowser(tab.linkedBrowser); + + let autoplay = permissions.find(({id}) => id === "autoplay-media"); + Assert.deepEqual(autoplay, { + id: "autoplay-media", + label: "Automatically Play Media with Sound", + state: SitePermissions.ALLOW, + scope: SitePermissions.SCOPE_TEMPORARY, + }); + + BrowserTestUtils.removeTab(gBrowser.selectedTab); +}); + // This tests the SitePermissions.getAllPermissionDetailsForBrowser function. add_task(async function testGetAllPermissionDetailsForBrowser() { let uri = Services.io.newURI("https://example.com");