Remove the "ask every time" policy for acceptance of cookies.

People still trying to use this option in 2017 will likely end up with a cookie confirmation dialog storm
that can end up with the browser being locked (modal dialog storm + confused z-order in some
window managers = inaccessible UI). This is a terrible footgun, and cookie acceptance is best
controlled with one of the many extensions available for this purpose.

This resolves #1475.

8 files changed, 33 insertions, 195 deletions

diff --git a/browser/components/preferences/privacy.js b/browser/components/preferences/privacy.js
index c3b9faf43..e291c781b 100644
--- a/browser/components/preferences/privacy.js
+++ b/browser/components/preferences/privacy.js
@@ -234,8 +234,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 :
@@ -383,7 +388,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/privacy.xul b/browser/components/preferences/privacy.xul
index 5290c3583..057d549a9 100644
--- a/browser/components/preferences/privacy.xul
+++ b/browser/components/preferences/privacy.xul
@@ -210,7 +210,6 @@
                         <menupopup>
                           <menuitem label="&expire.label;" value="0"/>
                           <menuitem label="&close.label;" value="2"/>
-                          <menuitem label="&askEachTime.label;" value="1"/>
                         </menupopup>
                       </menulist>
                     </hbox>
diff --git a/browser/locales/en-US/chrome/browser/preferences/privacy.dtd b/browser/locales/en-US/chrome/browser/preferences/privacy.dtd
index eae607092..92ebf93c4 100644
--- a/browser/locales/en-US/chrome/browser/preferences/privacy.dtd
+++ b/browser/locales/en-US/chrome/browser/preferences/privacy.dtd
@@ -38,7 +38,6 @@
 
 <!ENTITY  expire.label                  "they expire">
 <!ENTITY  close.label                   "I close &brandShortName;">
-<!ENTITY  askEachTime.label             "ask me every time">
 
 <!ENTITY  cookieExceptions.label        "Exceptions…">
 <!ENTITY  cookieExceptions.accesskey    "E">
diff --git a/dom/storage/DOMStorage.cpp b/dom/storage/DOMStorage.cpp
index aa143ce40..f74302a25 100644
--- a/dom/storage/DOMStorage.cpp
+++ b/dom/storage/DOMStorage.cpp
@@ -228,7 +228,6 @@ DOMStorage::BroadcastChangeNotification(const nsSubstring& aKey,
   NS_DispatchToMainThread(r);
 }
 
-static const uint32_t ASK_BEFORE_ACCEPT = 1;
 static const uint32_t ACCEPT_SESSION = 2;
 
 // Behavior pref constants taken from nsCookieService.cpp
@@ -287,8 +286,8 @@ DOMStorage::CanUseStorage(nsIDOMWindow* aWindow, DOMStorage* aStorage)
     uint32_t cookieBehavior = Preferences::GetUint(kCookiesBehavior);
     uint32_t lifetimePolicy = Preferences::GetUint(kCookiesLifetimePolicy);
 
-    // Treat "ask every time" as "reject always".
-    if ((cookieBehavior == BEHAVIOR_REJECT || lifetimePolicy == ASK_BEFORE_ACCEPT)) {
+    // Can't use DOM storage when policy is set to "reject always".
+    if (cookieBehavior == BEHAVIOR_REJECT) {
       return false;
     }
 
diff --git a/extensions/cookie/nsCookiePermission.cpp b/extensions/cookie/nsCookiePermission.cpp
index b7dd54bfd..99fadc520 100644
--- a/extensions/cookie/nsCookiePermission.cpp
+++ b/extensions/cookie/nsCookiePermission.cpp
@@ -31,7 +31,7 @@
 
 // values for mCookiesLifetimePolicy
 // 0 == accept normally
-// 1 == ask before accepting
+// 1 == ask before accepting -- obsolete, treated as ACCEPT_NORMALLY
 // 2 == downgrade to session
 // 3 == limit lifetime to N days
 static const uint32_t ACCEPT_NORMALLY = 0;
@@ -48,7 +48,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";
 
@@ -81,19 +80,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)
@@ -117,8 +108,13 @@ 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<int32_t>(ACCEPT_SESSION) &&
+        val != static_cast<int32_t>(ACCEPT_FOR_N_DAYS)) {
+      val = ACCEPT_NORMALLY;
+    }
     mCookiesLifetimePolicy = val;
+  }
 
   if (PREF_CHANGED(kCookiesLifetimeDays) &&
       NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimeDays, &val)))
@@ -235,127 +231,30 @@ nsCookiePermission::CanSetCookie(nsIURI     *aURI,
     break;
 
   default:
-    // the permission manager has nothing to say about this cookie -
-    // so, we apply the default prefs to it.
+    // The permission manager has nothing to say about this cookie
+    // so we apply the default prefs to it.
     NS_ASSERTION(perm == nsIPermissionManager::UNKNOWN_ACTION, "unknown permission");
 
-    // now we need to figure out what type of accept policy we're dealing with
-    // if we accept cookies normally, just bail and return
+    // Now we need to figure out what type of accept policy we're dealing with.
+    // If we accept cookies normally, just bail and return.
     if (mCookiesLifetimePolicy == ACCEPT_NORMALLY) {
       *aResult = true;
       return NS_OK;
     }
 
-    // declare this here since it'll be used in all of the remaining cases
+    // Declare this here since it'll be used in all of the remaining cases.
     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<nsICookiePromptService> 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<nsICookieManager2> 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 0d5c8ec38..794dce7d5 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 38a8832e0..000000000
--- a/extensions/cookie/test/test_bug1041808.html
+++ /dev/null
@@ -1,61 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=1041808
--->
-<head>
-  <meta charset="utf-8">
-  <title>Test for Bug 1041808</title>
-  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
-  <script type="application/javascript">
-
-  /** Test for Bug 1041808 **/
-
-SimpleTest.waitForExplicitFinish();
-
-var dialogsOpened = 0;
-var dialogsClosed = 0;
-function dismissDialog(aSubject, aTopic, aData)
-{
-  if (aTopic == "domwindowopened") {
-    var win = SpecialPowers.wrap(aSubject);
-    win.addEventListener("pageshow", function() {
-      win.removeEventListener("pageshow", arguments.callee, false);
-      sendKey("RETURN", aSubject);
-    }, false);
-    ++dialogsOpened;
-  } else if (aTopic == "domwindowclosed") {
-    ++dialogsClosed;
-  }
-}
-
-function runTest()
-{
-  SpecialPowers.Services.ww.registerNotification(dismissDialog);
-  document.cookie = "test1=testValue";
-  document.cookie = "test2=testValue";
-  document.cookie = "test3=testValue";
-  SpecialPowers.Services.ww.unregisterNotification(dismissDialog);
-  is(dialogsOpened, 3, "Setting a cookie should have asked for permission");
-  is(dialogsOpened - dialogsClosed, 0,
-     "Setting a cookie shouldn't have left any additional windows open");
-  SimpleTest.finish();
-}
-
-SpecialPowers.pushPrefEnv({"set": [["network.cookie.lifetimePolicy", 1]]},
-                          runTest);
-
-  </script>
-</head>
-<body>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1041808">Mozilla Bug 1041808</a>
-<p id="display"></p>
-<div id="content" style="display: none">
-
-</div>
-<pre id="test">
-</pre>
-</body>
-</html>
diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
index f4adaaaff..9f8533274 100644
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -2007,7 +2007,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 normally, 2-acceptForSession,3-acceptForNDays
 pref("network.cookie.alwaysAcceptSessionCookies", false);
 pref("network.cookie.prefsMigrated",        false);
 pref("network.cookie.lifetime.days",        90);