From 72753aa511c321f766301809f5285adcb120de3e Mon Sep 17 00:00:00 2001 From: M Hamdy Date: Thu, 8 Oct 2015 09:13:00 +0200 Subject: [PATCH] Bug 606655 - delete cookies UI option AskMeEveryTime and its related comments and tests. r=mak --- .../preferences/in-content/privacy.js | 10 +- .../preferences/in-content/privacy.xul | 1 - .../privatebrowsing/test/browser/browser.ini | 2 - ...er_privatebrowsing_cookieacceptdialog.html | 11 -- ...wser_privatebrowsing_cookieacceptdialog.js | 128 ----------------- .../chrome/browser/preferences/privacy.dtd | 1 - dom/base/nsContentUtils.cpp | 6 +- dom/storage/DOMStorage.cpp | 2 - .../test_localStorageCookieSettings.html | 15 -- extensions/cookie/nsCookiePermission.cpp | 136 +++--------------- extensions/cookie/test/mochitest.ini | 1 - extensions/cookie/test/test_bug1041808.html | 61 -------- modules/libpref/init/all.js | 2 +- netwerk/cookie/nsICookieService.idl | 4 +- netwerk/test/TestCookie.cpp | 2 - 15 files changed, 29 insertions(+), 353 deletions(-) delete mode 100644 browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.html delete mode 100644 browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.js delete mode 100644 extensions/cookie/test/test_bug1041808.html diff --git a/browser/components/preferences/in-content/privacy.js b/browser/components/preferences/in-content/privacy.js index 8a7c6ce92a5..3669605cbbf 100644 --- a/browser/components/preferences/in-content/privacy.js +++ b/browser/components/preferences/in-content/privacy.js @@ -274,8 +274,13 @@ var gPrivacyPane = { // adjust the cookie controls status this.readAcceptCookies(); - document.getElementById("keepCookiesUntil").value = disabled ? 2 : - document.getElementById("network.cookie.lifetimePolicy").value; + let lifetimePolicy = document.getElementById("network.cookie.lifetimePolicy").value; + if (lifetimePolicy != Ci.nsICookieService.ACCEPT_NORMALLY && + lifetimePolicy != Ci.nsICookieService.ACCEPT_SESSION && + lifetimePolicy != Ci.nsICookieService.ACCEPT_FOR_N_DAYS) { + lifetimePolicy = Ci.nsICookieService.ACCEPT_NORMALLY; + } + document.getElementById("keepCookiesUntil").value = disabled ? 2 : lifetimePolicy; // adjust the checked state of the sanitizeOnShutdown checkbox document.getElementById("alwaysClear").checked = disabled ? false : @@ -408,7 +413,6 @@ var gPrivacyPane = { * network.cookie.lifetimePolicy * - determines how long cookies are stored: * 0 means keep cookies until they expire - * 1 means ask how long to keep each cookie * 2 means keep cookies until the browser is closed */ diff --git a/browser/components/preferences/in-content/privacy.xul b/browser/components/preferences/in-content/privacy.xul index e12bc86996b..d09c2a6fd02 100644 --- a/browser/components/preferences/in-content/privacy.xul +++ b/browser/components/preferences/in-content/privacy.xul @@ -226,7 +226,6 @@ - diff --git a/browser/components/privatebrowsing/test/browser/browser.ini b/browser/components/privatebrowsing/test/browser/browser.ini index b07872e4295..3c10a2d4388 100644 --- a/browser/components/privatebrowsing/test/browser/browser.ini +++ b/browser/components/privatebrowsing/test/browser/browser.ini @@ -2,7 +2,6 @@ skip-if = buildapp == "mulet" support-files = browser_privatebrowsing_concurrent_page.html - browser_privatebrowsing_cookieacceptdialog.html browser_privatebrowsing_geoprompt_page.html browser_privatebrowsing_localStorage_before_after_page.html browser_privatebrowsing_localStorage_before_after_page2.html @@ -23,7 +22,6 @@ tags = trackingprotection [browser_privatebrowsing_cache.js] [browser_privatebrowsing_certexceptionsui.js] [browser_privatebrowsing_concurrent.js] -[browser_privatebrowsing_cookieacceptdialog.js] [browser_privatebrowsing_crh.js] [browser_privatebrowsing_downloadLastDir.js] [browser_privatebrowsing_downloadLastDir_c.js] diff --git a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.html b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.html deleted file mode 100644 index 80ac1bd642d..00000000000 --- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.html +++ /dev/null @@ -1,11 +0,0 @@ - - - - browser_privatebrowsing_cookieacceptdialog.html - - - - - diff --git a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.js b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.js deleted file mode 100644 index 98876ac54c6..00000000000 --- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.js +++ /dev/null @@ -1,128 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -// This test makes sure that private browsing mode disables the "remember" -// option in the cookie accept dialog. - -add_task(function* test() { - // initialization - const TEST_URL = "http://mochi.test:8888/browser/browser/components/" + - "privatebrowsing/test/browser/" + - "browser_privatebrowsing_cookieacceptdialog.html"; - const BLANK_URL = "http://mochi.test:8888/"; - let cp = Cc["@mozilla.org/embedcomp/cookieprompt-service;1"]. - getService(Ci.nsICookiePromptService); - - - function openCookieDialog(aWindow) { - let remember = {}; - const time = (new Date("Jan 1, 2030")).getTime() / 1000; - let cookie = { - name: "foo", - value: "bar", - isDomain: true, - host: "mozilla.org", - path: "/baz", - isSecure: false, - expires: time, - status: 0, - policy: 0, - isSession: false, - expiry: time, - isHttpOnly: true, - QueryInterface: function(iid) { - const validIIDs = [Ci.nsISupports, Ci.nsICookie, Ci.nsICookie2]; - for (var i = 0; i < validIIDs.length; ++i) - if (iid == validIIDs[i]) - return this; - throw Cr.NS_ERROR_NO_INTERFACE; - } - }; - - executeSoon(function () { - cp.cookieDialog(aWindow, cookie, "mozilla.org", 10, false, remember); - }); - return BrowserTestUtils.domWindowOpened(); - }; - - - function checkRememberOption(expectedDisabled, aWindow) { - return Task.spawn(function* () { - let dialogWin = yield openCookieDialog(aWindow); - - yield new Promise(resolve => { - dialogWin.addEventListener("load", function onLoad(event) { - dialogWin.removeEventListener("load", onLoad, false); - resolve(); - }, false); - }); - - let doc = dialogWin.document; - let remember = doc.getElementById("persistDomainAcceptance"); - ok(remember, "The remember checkbox should exist"); - - if (expectedDisabled) - is(remember.getAttribute("disabled"), "true", - "The checkbox should be disabled"); - else - ok(!remember.hasAttribute("disabled"), - "The checkbox should not be disabled"); - - yield BrowserTestUtils.closeWindow(dialogWin); - }); - }; - - function checkSettingDialog(aIsPrivateWindow, aWindow) { - return Task.spawn(function* () { - let dialogOpened = false; - let promiseDialogClosed = null; - - function observer(subject, topic, data) { - if (topic != "domwindowopened") { return; } - Services.ww.unregisterNotification(observer); - dialogOpened = true; - - promiseDialogClosed = BrowserTestUtils.closeWindow( - subject.QueryInterface(Ci.nsIDOMWindow)); - } - Services.ww.registerNotification(observer); - - let selectedBrowser = aWindow.gBrowser.selectedBrowser; - selectedBrowser.loadURI(TEST_URL); - yield BrowserTestUtils.browserLoaded(selectedBrowser);; - - if (dialogOpened) { - ok(!aIsPrivateWindow, - "Setting dialog shown, confirm normal window"); - } else { - Services.ww.unregisterNotification(observer); - ok(aIsPrivateWindow, - "Confirm setting dialog is not displayed for private window"); - } - - yield promiseDialogClosed; - }); - }; - - // Ask all cookies - Services.prefs.setIntPref("network.cookie.lifetimePolicy", 1); - - let win = yield BrowserTestUtils.openNewBrowserWindow(); - info("Test on public window"); - - yield checkRememberOption(false, win); - yield checkSettingDialog(false, win); - - let privateWin = yield BrowserTestUtils.openNewBrowserWindow({private: true}); - info("Test on private window"); - - yield checkRememberOption(true, privateWin); - yield checkSettingDialog(true, privateWin); - - - // Cleanup - Services.prefs.clearUserPref("network.cookie.lifetimePolicy"); - yield BrowserTestUtils.closeWindow(win); - yield BrowserTestUtils.closeWindow(privateWin); -}); diff --git a/browser/locales/en-US/chrome/browser/preferences/privacy.dtd b/browser/locales/en-US/chrome/browser/preferences/privacy.dtd index 830dd46330e..cb2dce6e8e6 100644 --- a/browser/locales/en-US/chrome/browser/preferences/privacy.dtd +++ b/browser/locales/en-US/chrome/browser/preferences/privacy.dtd @@ -47,7 +47,6 @@ - diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 8a892a0c4a8..9943ca8f80a 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -8233,10 +8233,8 @@ nsContentUtils::InternalStorageAllowedForPrincipal(nsIPrincipal* aPrincipal, return access; } - // We don't want to prompt for every attempt to access permissions, so we - // treat the cookie ASK_BEFORE_ACCEPT as though it was a reject. - if (sCookiesBehavior == nsICookieService::BEHAVIOR_REJECT || - sCookiesLifetimePolicy == nsICookieService::ASK_BEFORE_ACCEPT) { + // We don't want to prompt for every attempt to access permissions. + if (sCookiesBehavior == nsICookieService::BEHAVIOR_REJECT) { return StorageAccess::eDeny; } diff --git a/dom/storage/DOMStorage.cpp b/dom/storage/DOMStorage.cpp index 4e8e7b2fcf0..d52f27b41df 100644 --- a/dom/storage/DOMStorage.cpp +++ b/dom/storage/DOMStorage.cpp @@ -232,8 +232,6 @@ DOMStorage::BroadcastChangeNotification(const nsSubstring& aKey, static const char kPermissionType[] = "cookie"; static const char kStorageEnabled[] = "dom.storage.enabled"; -static const char kCookiesBehavior[] = "network.cookie.cookieBehavior"; -static const char kCookiesLifetimePolicy[] = "network.cookie.lifetimePolicy"; // static, public bool diff --git a/dom/tests/mochitest/localstorage/test_localStorageCookieSettings.html b/dom/tests/mochitest/localstorage/test_localStorageCookieSettings.html index d3a47611e1f..99f3b1acef8 100644 --- a/dom/tests/mochitest/localstorage/test_localStorageCookieSettings.html +++ b/dom/tests/mochitest/localstorage/test_localStorageCookieSettings.html @@ -27,21 +27,6 @@ function test1() { is(ex.name, "SecurityError"); } - // Set cookies behavior to "ask every time". - SpecialPowers.pushPrefEnv({"set": [["network.cookie.lifetimePolicy", 1]], - "clear": [["network.cookie.cookieBehavior"]]}, - test2); -} - -function test2() { - try { - localStorage.setItem("contentkey", "test-value"); - ok(false, "Setting localStorageItem should throw a security exception"); - } - catch(ex) { - is(ex.name, "SecurityError"); - } - // Set cookies behavior to "reject 3rd party" SpecialPowers.pushPrefEnv({"set": [["network.cookie.cookieBehavior", 1]], "clear": [["network.cookie.lifetimePolicy"]]}, diff --git a/extensions/cookie/nsCookiePermission.cpp b/extensions/cookie/nsCookiePermission.cpp index 065dbe746cb..86e84b7f2db 100644 --- a/extensions/cookie/nsCookiePermission.cpp +++ b/extensions/cookie/nsCookiePermission.cpp @@ -34,7 +34,7 @@ // values for mCookiesLifetimePolicy // 0 == accept normally -// 1 == ask before accepting +// 1 == ask before accepting, no more supported, treated like ACCEPT_NORMALLY (Bug 606655). // 2 == downgrade to session // 3 == limit lifetime to N days static const uint32_t ACCEPT_NORMALLY = 0; @@ -51,7 +51,6 @@ static const char kCookiesPrefsMigrated[] = "network.cookie.prefsMigrated"; // obsolete pref names for migration static const char kCookiesLifetimeEnabled[] = "network.cookie.lifetime.enabled"; static const char kCookiesLifetimeBehavior[] = "network.cookie.lifetime.behavior"; -static const char kCookiesAskPermission[] = "network.cookie.warnAboutCookies"; static const char kPermissionType[] = "cookie"; @@ -84,19 +83,11 @@ nsCookiePermission::Init() bool migrated; rv = prefBranch->GetBoolPref(kCookiesPrefsMigrated, &migrated); if (NS_FAILED(rv) || !migrated) { - bool warnAboutCookies = false; - prefBranch->GetBoolPref(kCookiesAskPermission, &warnAboutCookies); - - // if the user is using ask before accepting, we'll use that - if (warnAboutCookies) - prefBranch->SetIntPref(kCookiesLifetimePolicy, ASK_BEFORE_ACCEPT); - bool lifetimeEnabled = false; prefBranch->GetBoolPref(kCookiesLifetimeEnabled, &lifetimeEnabled); - - // if they're limiting lifetime and not using the prompts, use the - // appropriate limited lifetime pref - if (lifetimeEnabled && !warnAboutCookies) { + + // if they're limiting lifetime, use the appropriate limited lifetime pref + if (lifetimeEnabled) { int32_t lifetimeBehavior; prefBranch->GetIntPref(kCookiesLifetimeBehavior, &lifetimeBehavior); if (lifetimeBehavior) @@ -120,8 +111,12 @@ nsCookiePermission::PrefChanged(nsIPrefBranch *aPrefBranch, #define PREF_CHANGED(_P) (!aPref || !strcmp(aPref, _P)) if (PREF_CHANGED(kCookiesLifetimePolicy) && - NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimePolicy, &val))) + NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimePolicy, &val))) { + if (val != static_cast(ACCEPT_SESSION) && val != static_cast(ACCEPT_FOR_N_DAYS)) { + val = ACCEPT_NORMALLY; + } mCookiesLifetimePolicy = val; + } if (PREF_CHANGED(kCookiesLifetimeDays) && NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimeDays, &val))) @@ -253,112 +248,15 @@ nsCookiePermission::CanSetCookie(nsIURI *aURI, int64_t currentTime = PR_Now() / PR_USEC_PER_SEC; int64_t delta = *aExpiry - currentTime; - // check whether the user wants to be prompted - if (mCookiesLifetimePolicy == ASK_BEFORE_ACCEPT) { - // if it's a session cookie and the user wants to accept these - // without asking, or if we are in private browsing mode, just - // accept the cookie and return - if ((*aIsSession && mCookiesAlwaysAcceptSession) || - (aChannel && NS_UsePrivateBrowsing(aChannel))) { - *aResult = true; - return NS_OK; - } - - // default to rejecting, in case the prompting process fails - *aResult = false; - - nsAutoCString hostPort; - aURI->GetHostPort(hostPort); - - if (!aCookie) { - return NS_ERROR_UNEXPECTED; - } - // If there is no host, use the scheme, and append "://", - // to make sure it isn't a host or something. - // This is done to make the dialog appear for javascript cookies from - // file:// urls, and make the text on it not too weird. (bug 209689) - if (hostPort.IsEmpty()) { - aURI->GetScheme(hostPort); - if (hostPort.IsEmpty()) { - // still empty. Just return the default. - return NS_OK; - } - hostPort = hostPort + NS_LITERAL_CSTRING("://"); - } - - // we don't cache the cookiePromptService - it's not used often, so not - // worth the memory. - nsresult rv; - nsCOMPtr cookiePromptService = - do_GetService(NS_COOKIEPROMPTSERVICE_CONTRACTID, &rv); - if (NS_FAILED(rv)) return rv; - - // get some useful information to present to the user: - // whether a previous cookie already exists, and how many cookies this host - // has set - bool foundCookie = false; - uint32_t countFromHost; - nsCOMPtr cookieManager = do_GetService(NS_COOKIEMANAGER_CONTRACTID, &rv); - if (NS_SUCCEEDED(rv)) { - nsAutoCString rawHost; - aCookie->GetRawHost(rawHost); - rv = cookieManager->CountCookiesFromHost(rawHost, &countFromHost); - - if (NS_SUCCEEDED(rv) && countFromHost > 0) - rv = cookieManager->CookieExists(aCookie, &foundCookie); - } - if (NS_FAILED(rv)) return rv; - - // check if the cookie we're trying to set is already expired, and return; - // but only if there's no previous cookie, because then we need to delete the previous - // cookie. we need this check to avoid prompting the user for already-expired cookies. - if (!foundCookie && !*aIsSession && delta <= 0) { - // the cookie has already expired. accept it, and let the backend figure - // out it's expired, so that we get correct logging & notifications. - *aResult = true; - return rv; - } - - bool rememberDecision = false; - int32_t dialogRes = nsICookiePromptService::DENY_COOKIE; - rv = cookiePromptService->CookieDialog(nullptr, aCookie, hostPort, - countFromHost, foundCookie, - &rememberDecision, &dialogRes); - if (NS_FAILED(rv)) return rv; - - *aResult = !!dialogRes; - if (dialogRes == nsICookiePromptService::ACCEPT_SESSION_COOKIE) + // We are accepting the cookie, but, + // if it's not a session cookie, we may have to limit its lifetime. + if (!*aIsSession && delta > 0) { + if (mCookiesLifetimePolicy == ACCEPT_SESSION) { + // limit lifetime to session *aIsSession = true; - - if (rememberDecision) { - switch (dialogRes) { - case nsICookiePromptService::DENY_COOKIE: - mPermMgr->Add(aURI, kPermissionType, (uint32_t) nsIPermissionManager::DENY_ACTION, - nsIPermissionManager::EXPIRE_NEVER, 0); - break; - case nsICookiePromptService::ACCEPT_COOKIE: - mPermMgr->Add(aURI, kPermissionType, (uint32_t) nsIPermissionManager::ALLOW_ACTION, - nsIPermissionManager::EXPIRE_NEVER, 0); - break; - case nsICookiePromptService::ACCEPT_SESSION_COOKIE: - mPermMgr->Add(aURI, kPermissionType, nsICookiePermission::ACCESS_SESSION, - nsIPermissionManager::EXPIRE_NEVER, 0); - break; - default: - break; - } - } - } else { - // we're not prompting, so we must be limiting the lifetime somehow - // if it's a session cookie, we do nothing - if (!*aIsSession && delta > 0) { - if (mCookiesLifetimePolicy == ACCEPT_SESSION) { - // limit lifetime to session - *aIsSession = true; - } else if (delta > mCookiesLifetimeSec) { - // limit lifetime to specified time - *aExpiry = currentTime + mCookiesLifetimeSec; - } + } else if (delta > mCookiesLifetimeSec) { + // limit lifetime to specified time + *aExpiry = currentTime + mCookiesLifetimeSec; } } } diff --git a/extensions/cookie/test/mochitest.ini b/extensions/cookie/test/mochitest.ini index 0d5c8ec3832..794dce7d52e 100644 --- a/extensions/cookie/test/mochitest.ini +++ b/extensions/cookie/test/mochitest.ini @@ -38,4 +38,3 @@ support-files = [test_same_base_domain_5.html] [test_same_base_domain_6.html] [test_samedomain.html] -[test_bug1041808.html] diff --git a/extensions/cookie/test/test_bug1041808.html b/extensions/cookie/test/test_bug1041808.html deleted file mode 100644 index 38a8832e0e9..00000000000 --- a/extensions/cookie/test/test_bug1041808.html +++ /dev/null @@ -1,61 +0,0 @@ - - - - - - Test for Bug 1041808 - - - - - - -Mozilla Bug 1041808 -

- -
-
- - diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 8ec45ef8060..72c640384f1 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1819,7 +1819,7 @@ pref("network.cookie.cookieBehavior", 0); // 0-Accept, 1-dontAcceptForeign pref("network.cookie.cookieBehavior", 0); // Keep the old default of accepting all cookies #endif pref("network.cookie.thirdparty.sessionOnly", false); -pref("network.cookie.lifetimePolicy", 0); // accept normally, 1-askBeforeAccepting, 2-acceptForSession,3-acceptForNDays +pref("network.cookie.lifetimePolicy", 0); // 0-accept, 2-acceptForSession, 3-acceptForNDays pref("network.cookie.alwaysAcceptSessionCookies", false); pref("network.cookie.prefsMigrated", false); pref("network.cookie.lifetime.days", 90); diff --git a/netwerk/cookie/nsICookieService.idl b/netwerk/cookie/nsICookieService.idl index 678d083f970..053fad6db15 100644 --- a/netwerk/cookie/nsICookieService.idl +++ b/netwerk/cookie/nsICookieService.idl @@ -71,7 +71,7 @@ interface nsIChannel; * to set the cookie. * data : the referrer, or "?" if unknown */ -[scriptable, uuid(f5807c53-de48-461a-8117-bd156bc2dcf0)] +[scriptable, uuid(1e94e283-2811-4f43-b947-d22b1549d824)] interface nsICookieService : nsISupports { /* @@ -87,7 +87,7 @@ interface nsICookieService : nsISupports * Possible values for the "network.cookie.lifetimePolicy" preference. */ const uint32_t ACCEPT_NORMALLY = 0; // accept normally - const uint32_t ASK_BEFORE_ACCEPT = 1; // ask before accepting + // Value = 1 is considered the same as 0 (See Bug 606655). const uint32_t ACCEPT_SESSION = 2; // downgrade to session const uint32_t ACCEPT_FOR_N_DAYS = 3; // limit lifetime to N days diff --git a/netwerk/test/TestCookie.cpp b/netwerk/test/TestCookie.cpp index 217ef79b577..e123ec66b25 100644 --- a/netwerk/test/TestCookie.cpp +++ b/netwerk/test/TestCookie.cpp @@ -28,7 +28,6 @@ static const char kCookiesPermissions[] = "network.cookie.cookieBehavior"; static const char kCookiesLifetimeEnabled[] = "network.cookie.lifetime.enabled"; static const char kCookiesLifetimeDays[] = "network.cookie.lifetime.days"; static const char kCookiesLifetimeCurrentSession[] = "network.cookie.lifetime.behavior"; -static const char kCookiesAskPermission[] = "network.cookie.warnAboutCookies"; static const char kCookiesMaxPerHost[] = "network.cookie.maxPerHost"; static char *sBuffer; @@ -183,7 +182,6 @@ InitPrefs(nsIPrefBranch *aPrefBranch) aPrefBranch->SetBoolPref(kCookiesLifetimeEnabled, true); aPrefBranch->SetIntPref(kCookiesLifetimeCurrentSession, 0); aPrefBranch->SetIntPref(kCookiesLifetimeDays, 1); - aPrefBranch->SetBoolPref(kCookiesAskPermission, false); // Set the base domain limit to 50 so we have a known value. aPrefBranch->SetIntPref(kCookiesMaxPerHost, 50); }