Remove "Ask me everytime" cookies option.

VERIFIED FIXED in Firefox 44

Status

()

Firefox
Preferences
VERIFIED FIXED
7 years ago
7 months ago

People

(Reporter: baffclan, Assigned: M Hamdy)

Tracking

({dev-doc-needed, useless-UI})

Trunk
Firefox 44
dev-doc-needed, useless-UI
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 verified)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

7 years ago
Tool -> Options -> Privacy
"ask me every time" in "Keep until"

see bug 570366 comment 1
Boris Zbarsky said :
> This option isn't supported, last I checked; I have no idea why the UI keeps
> exposing it....
(Reporter)

Comment 1

6 years ago
-> INVALID
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INVALID

Comment 2

6 years ago
This seems valid to me....
Status: RESOLVED → REOPENED
Resolution: INVALID → ---

Updated

6 years ago
Keywords: useless-UI
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [good first bug]
Some questions:
- What should the UI show for users who have this pref set, once we remove the option?
- Will the backend be removed? If so, what will the backend behavior be for those users when it is removed?

Comment 4

6 years ago
I think the backend should be removed, yes.  Not sure what the right behavior should be there...

Note that any users who set the pref are probably crashing multiple times a day as a result.... so I expect the number of such users who are still using the browser is pretty low.
Created attachment 582432 [details] [diff] [review]
Remove UI

Do you want me to remove backend as well?
(In reply to Thomas Prip Vestergaard [:prip] from comment #5)
> Created attachment 582432 [details] [diff] [review]
> Remove UI
> 
> Do you want me to remove backend as well?

I think this can happen in a followup bug, unless you want to do it here.
Assignee: nobody → thomas
(In reply to Dão Gottwald [:dao] from comment #6)
> > Do you want me to remove backend as well?
> 
> I think this can happen in a followup bug, unless you want to do it here.

I will give it a try next week!
Thomas, is this patch ready for review?
(Assignee)

Comment 9

2 years ago
Hello,

What's the status of this issue?! I think I can start with this bug.
If it needs more work, plz assign it to me.

Thanks
Assignee: thomas → nobody
Assignee: nobody → m.hamdy88
(Assignee)

Comment 10

2 years ago
@Thomas , I found no logic behind keepCookiesUntil.value =1 , but I found some tests and comments, do you want me to remove these tests and comments?

Thanks
(Assignee)

Updated

2 years ago
Flags: needinfo?(thomas)
@M Hamdy
I don't know anything about this bug - sorry.

Please try and ask Boris Zbarsky or Dão Gottwald.
Flags: needinfo?(thomas)
(Assignee)

Updated

2 years ago
Flags: needinfo?(dao)
Flags: needinfo?(bzbarsky)

Comment 12

2 years ago
The tests can comments can go if the implementation is gone too.
Flags: needinfo?(bzbarsky)

Updated

2 years ago
Flags: needinfo?(dao)
(Assignee)

Comment 13

2 years ago
Created attachment 8659321 [details] [diff] [review]
Delete cookies UI option AskMeEveryTime and its related comments and tests

Please pay more attention to changes at these files:
test_localStorageCookieSettings
browser_privatebrowsing_cookieacceptdialog
Attachment #8659321 - Flags: review?(mak77)
(Assignee)

Updated

2 years ago
Attachment #8659321 - Flags: review?(gavin.sharp)
(Assignee)

Updated

2 years ago
Attachment #8659321 - Flags: review?(bzbarsky)
The questions from comment 3 still need an answer.

It looks like the patch removes ASK_BEFORE_ACCEPT from the IDL, but not its uses elsewhere (see https://mxr.mozilla.org/mozilla-central/search?string=ASK_BEFORE_ACCEPT, nsContentUtils.cpp and nsCookiePermission.cpp).
Comment on attachment 8659321 [details] [diff] [review]
Delete cookies UI option AskMeEveryTime and its related comments and tests

It seems like one possible answer for those questions could be "treat pref=1 the same as pref=0" (both in the UI and in the backend). That would require making the right changes to the backend and then making sure the UI properly reflects that behavior.

Someone on #fx-team may be able to help you more actively, I'm no longer active reviewing Mozilla patches.
Attachment #8659321 - Flags: review?(mak77)
Attachment #8659321 - Flags: review?(gavin.sharp)
Attachment #8659321 - Flags: review?(bzbarsky)
Attachment #8659321 - Flags: review-
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #15)
> Comment on attachment 8659321 [details] [diff] [review]
> It seems like one possible answer for those questions could be "treat pref=1
> the same as pref=0" (both in the UI and in the backend). That would require
> making the right changes to the backend and then making sure the UI properly
> reflects that behavior.

I think this is a good idea (better than trying to migrate the pref and then trying to intercept any try to change the pref to "1"), provided we mark in the idl to NOT reuse the value 1.
(Assignee)

Comment 17

2 years ago
Now I got lost, Please could u clarify what's needed exactly :) ?
Flags: needinfo?(mak77)
(In reply to M Hamdy from comment #17)
> Now I got lost, Please could u clarify what's needed exactly :) ?

sorry if it was unclear.
First, you need to actually remove the backend code, see comment 14. There is code actually using that idl constant. Also, even if theoretically it is not needed for constants, please bump the idl UUID cause I think we have a push hook that will fail tests if you don't.

Then we also need to handle users who had the pref set to 1, and now they would get an invalid "lifetime" in the backend. Gavin suggested to make the backend consider 1 the same as 0, and I agree it's very likely a good idea. But there's a risk, if anyone in future reintroduces "1" as a valid value in the idl, the code will go mad. So I suggest replacing the constant in the idl with a comment explaining to never add back the value "1" cause it's considered the same as 0.

Does this help?
Flags: needinfo?(mak77)
(Assignee)

Comment 19

2 years ago
Created attachment 8663339 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

@Marco Bonardo

This is what I've done at this patch:
-Remove the backend code related to ASK_BEFORE_ACCEPT case.
-Add comments reserving value 1 not to be used in the future (at idl , nsCookiePermission.cpp files)
-Treat ASK_BEFORE_ACCEPT like ACCEPT_NORMALLY

Please review and let me know if any additional work is needed...
waiting your feedback.
Thanks
Attachment #8659321 - Attachment is obsolete: true
Flags: needinfo?(mak77)
Attachment #8663339 - Flags: review?(mak77)
Comment on attachment 8663339 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

Review of attachment 8663339 [details] [diff] [review]:
-----------------------------------------------------------------

Look great! There are still some things that need to be fixed, but nice progress.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_cookieacceptdialog.js
@@ +105,5 @@
>      });
>    };
>  
>    // Ask all cookies
> +  Services.prefs.setIntPref("network.cookie.lifetimePolicy", 0);

I think we can completely remove this test. Its scope is to check we modify the accept cookie dialog when in private browsing mode, but since we are removing the code for the accept cookie dialog, this test doesn't make anymore sense.
Remember to also remove it from the corresponding browser.ini file.

::: dom/tests/mochitest/localstorage/test_localStorageCookieSettings.html
@@ +24,5 @@
>      ok(false, "Setting localStorageItem should throw a security exception");
>    }
>    catch(ex) {
>      is(ex.name, "SecurityError");
>    }

the change here made me think we may have code in localstorage handling this pref, so I found this
http://mxr.mozilla.org/mozilla-central/search?string=kCookies&find=%2Fstorage
Both look unused and can likely be removed.

@@ +37,1 @@
>  function test3() {

the added newline here is not necessary

::: extensions/cookie/nsCookiePermission.cpp
@@ +33,5 @@
>   ****************************************************************/
>  
>  // values for mCookiesLifetimePolicy
>  // 0 == accept normally
> +// 1 == ask before accepting, =>Bug606655, it's decided to be treated like ACCEPT_NORMALLY.

", no more supported, treated like ACCEPT_NORMALLY (Bug 606655)."

@@ +38,4 @@
>  // 2 == downgrade to session
>  // 3 == limit lifetime to N days
>  static const uint32_t ACCEPT_NORMALLY = 0;
> +static const uint32_t ASK_BEFORE_ACCEPT = 1; //Bug606655, it's decided to be treated like ACCEPT_NORMALLY.

same comment...

@@ +84,5 @@
>      bool migrated;
>      rv = prefBranch->GetBoolPref(kCookiesPrefsMigrated, &migrated);
>      if (NS_FAILED(rv) || !migrated) {
>        bool warnAboutCookies = false;
>        prefBranch->GetBoolPref(kCookiesAskPermission, &warnAboutCookies);

warnAboutCookies was the previous version of ASK_BEFORE_ACCEPT, so we could also remove it... but basically we could remove all of kCookiesPrefsMigrated and related code since it's ancient, and the patch risks to become too large if we do...
I think, if you wish, after this bug you could file a new separate bug and work on removing cookie prefs migration code.
Here we won't do that.

@@ +241,5 @@
>      // now we need to figure out what type of accept policy we're dealing with
>      // if we accept cookies normally, just bail and return
> +    // as Bug606655, ASK_BEFORE_ACCEPT is decided to be treated like ACCEPT_NORMALLY.
> +    if (mCookiesLifetimePolicy == ACCEPT_NORMALLY ||
> +          mCookiesLifetimePolicy == ASK_BEFORE_ACCEPT) {

rather than doing this, that is error-prone to future changes, I think you should intercept the point where we set mCookiesLifetimePolicy (here! http://mxr.mozilla.org/mozilla-central/source/extensions/cookie/nsCookiePermission.cpp#124) and replace ASK_BEFORE_ACCEPT with ACCEPT_NORMALLY
Even better if you make so that any unrecognized value (anything that is not 2 or 3) gets converted to ACCEPT_NORMALLY.

@@ +252,5 @@
>      int64_t delta = *aExpiry - currentTime;
>  
> +   
> +    // we're not prompting, so we must be limiting the lifetime somehow
> +    // if it's a session cookie, we do nothing

trailing spaces and too many newlines.

I'd also rephrase to something like:
"We are accepting the cookie, but, if it's not a session cookie, we may have to limit its lifetime."

@@ +264,3 @@
>        }
>      }
> +    

trailing spaces

::: extensions/cookie/test/test_bug1041808.html
@@ -1,1 @@
> -<!DOCTYPE HTML>

when removing a test you must also remove it from the manifest: http://mxr.mozilla.org/mozilla-central/source/extensions/cookie/test/mochitest.ini#41

::: modules/libpref/init/all.js
@@ +1812,5 @@
>  #ifdef ANDROID
>  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,2-acceptForSession,3-acceptForNDays

please change "accept normally" to just "0-accept" and add back whitespaces after the commas.

::: netwerk/cookie/nsICookieService.idl
@@ +85,5 @@
>  
>    /*
>     * Possible values for the "network.cookie.lifetimePolicy" preference.
>     */
>    const uint32_t ACCEPT_NORMALLY   = 0; // accept normally

it's ideally not necessary to rev the uuid when changing constants, but IIRC we now have a push hook that fails if you don't rev the uuid every time you change an idl, so please just do that.

@@ +86,5 @@
>    /*
>     * Possible values for the "network.cookie.lifetimePolicy" preference.
>     */
>    const uint32_t ACCEPT_NORMALLY   = 0; // accept normally
> +                                        //Value = 1 is considered the same as 0 Bug606655

you can just align this to the left, since it's a "special" comment replacing one line, also please add whitespace after // and s/Bug606655/(See Bug 606655)./
Attachment #8663339 - Flags: review?(mak77)

Updated

2 years ago
Flags: needinfo?(mak77)
(Assignee)

Comment 21

2 years ago
Created attachment 8665750 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

@Marco Bonardo

This is what I've done at this patch:

1- Removing browser_privatebrowsing_cookieacceptdialog.js/browser_privatebrowsing_cookieacceptdialog.html and removing them from browser.ini file.
2- Removing unneeded new line at 'test_localStorageCookieSettings.html'.
3- Removing 'kCookiesLifetimePolicy' and 'kCookiesBehavior' from 'DOMStorage.cpp' file.
4- Change the comments at 'nsCookiePermission.cpp' file.
5- I didn't get exactly what is needed about 'WarnAboutCookies', plz pay more attention to my changes there.
6- Intercepting 'mCookiesLifetimePolicy'and replace it with ACCEPT_NORMALLY if it's not (2 or 3).
7- Handling the trailing spaces and unneeded new lines at 'nsCookiePermission.cpp'.
8- Removing 'test_bug1041808.html' from the manifest.
9- Comments are changed at 'all.js' file.
10- uuid has been changed
11- Comment changed at 'nsICookieService.idl' file.


Please review and let me know if any additional work is needed...
waiting your feedback.
Thanks
Attachment #8663339 - Attachment is obsolete: true
Flags: needinfo?(mak77)
Attachment #8665750 - Flags: review?(mak77)
just a side note, you don't need to ask for both needinfo and review, one flag is enough to make the request appear in the bugzilla dashboard. Usually needinfo is used when you need info but are not attaching a patch.

(In reply to M Hamdy from comment #21)
> 5- I didn't get exactly what is needed about 'WarnAboutCookies', plz pay
> more attention to my changes there.

Sorry if it was unclear, there was nothing to do. Basically we need to fix it, but in a separate bug.
Flags: needinfo?(mak77)
(Assignee)

Comment 23

2 years ago
Ok, I got it :-) sry for inconvenience, I put that flag as I needed info about point #5 :)  sry agian. 

About the patch,  is it ok? I could have some freetime  tonight if any additional work is needed. 
Thanks
Comment on attachment 8665750 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

Review of attachment 8665750 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry if it took a while.

Apart from these fixes, I think it would be nice if you could file 2 new bugs (and if you wish you could work on them once this is done):
1. Remove the migration code, that is basically everything regarding kCookiesPrefsMigrated
2. remove ACCEPT_FOR_N_DAYS option. I honestly don't see what's the point of this unused option. We can ask around for some agreement on doing that, but I don't expect roadblocks...

Also, this MDN article needs to be updated: https://developer.mozilla.org/it/docs/Cookies_Preferences_in_Mozilla

::: browser/components/preferences/in-content/privacy.js
@@ -391,5 @@
>     *         see netwerk/cookie/src/nsCookieService.cpp for details
>     * 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

the dialog could not actually represent the current status of the pref, if a user has the pref set to 1, we will accept normally but the dialog won't show that.
We need to fix the preferences code too:

http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content/privacy.js#277
277       document.getElementById("keepCookiesUntil").value = disabled ? 2 :
278         document.getElementById("network.cookie.lifetimePolicy").value;

should be something like:
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 = nsICookieService::ACCEPT_NORMALLY;
}
document.getElementById("keepCookiesUntil").value = disabled ? 2 : lifetimePolicy;

::: extensions/cookie/nsCookiePermission.cpp
@@ +38,4 @@
>  // 2 == downgrade to session
>  // 3 == limit lifetime to N days
>  static const uint32_t ACCEPT_NORMALLY = 0;
> +static const uint32_t ASK_BEFORE_ACCEPT = 1; //no more supported, treated like ACCEPT_NORMALLY (Bug 606655).

please add space after //

@@ +83,5 @@
>      // migration code for original cookie prefs
>      bool migrated;
>      rv = prefBranch->GetBoolPref(kCookiesPrefsMigrated, &migrated);
>      if (NS_FAILED(rv) || !migrated) {
> +              

please remove trailing spaces

@@ +88,1 @@
>        bool lifetimeEnabled = false;

you can remove kCookiesAskPermission from the top of this file.
and also from:
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/TestCookie.cpp#31
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/TestCookie.cpp#186

@@ +112,5 @@
>  
>  #define PREF_CHANGED(_P) (!aPref || !strcmp(aPref, _P))
>  
>    if (PREF_CHANGED(kCookiesLifetimePolicy) &&
> +      NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimePolicy, &val))){

please add space before {

@@ +114,5 @@
>  
>    if (PREF_CHANGED(kCookiesLifetimePolicy) &&
> +      NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimePolicy, &val))){
> +    if(val != ACCEPT_SESSION && val != ACCEPT_FOR_N_DAYS)
> +      val = ACCEPT_NORMALLY;

please add space after if and brace the if body.
The coding style is
if (condition) {
  body
}

@@ +115,5 @@
>    if (PREF_CHANGED(kCookiesLifetimePolicy) &&
> +      NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimePolicy, &val))){
> +    if(val != ACCEPT_SESSION && val != ACCEPT_FOR_N_DAYS)
> +      val = ACCEPT_NORMALLY;
> +    mCookiesLifetimePolicy = val;    

please remove trailing spaces
Attachment #8665750 - Flags: review?(mak77)
(Assignee)

Comment 25

2 years ago
Created attachment 8669327 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

done.
Attachment #8669327 - Flags: review?(mak77)
(Assignee)

Updated

2 years ago
Attachment #8665750 - Attachment is obsolete: true

Updated

2 years ago
Blocks: 430006

Updated

2 years ago
Attachment #582432 - Attachment is obsolete: true
Comment on attachment 8669327 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

Review of attachment 8669327 [details] [diff] [review]:
-----------------------------------------------------------------

It looks mostly good to me, there's still some small things to cleanup and one mistake.
I will push your next patch to the Try Server as soon as you attach it.
Sorry if this was a slower than expected process.

::: browser/components/preferences/in-content/privacy.js
@@ +273,5 @@
>  
>        // adjust the cookie controls status
>        this.readAcceptCookies();
> +      let lifetimePolicy = document.getElementById("network.cookie.lifetimePolicy").value;
> +      if (lifetimePolicy != Ci.nsICookieService.ACCEPT_NORMALLY&&

there is a missing whitespace after NORMALLY and before &&.

Are you rebuilding Firefox and testing the privacy panel to check it's working as expected?

::: extensions/cookie/nsCookiePermission.cpp
@@ +38,4 @@
>  // 2 == downgrade to session
>  // 3 == limit lifetime to N days
>  static const uint32_t ACCEPT_NORMALLY = 0;
> +static const uint32_t ASK_BEFORE_ACCEPT = 1; // no more supported, treated like ACCEPT_NORMALLY (Bug 606655).

Sorry, since the above comment already states what each value means, I think we can remove the comment from this line.

@@ +81,5 @@
>  
>      // migration code for original cookie prefs
>      bool migrated;
>      rv = prefBranch->GetBoolPref(kCookiesPrefsMigrated, &migrated);
> +    if (NS_FAILED(rv) || !migrated) {   

please remove trailing spaces from here

@@ +112,5 @@
>  
>    if (PREF_CHANGED(kCookiesLifetimePolicy) &&
> +      NS_SUCCEEDED(aPrefBranch->GetIntPref(kCookiesLifetimePolicy, &val))) {
> +    if (val != ACCEPT_SESSION && val != ACCEPT_FOR_N_DAYS)
> +      val = ACCEPT_NORMALLY;

please brace this if
Attachment #8669327 - Flags: review?(mak77) → feedback+
(Assignee)

Comment 27

2 years ago
Created attachment 8670845 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

- privacy.js: done, yes I tried last time to build and check it, but I did't cover all the code path of changes.
- nsCookiePermission.cpp: done.
Attachment #8669327 - Attachment is obsolete: true
Attachment #8670845 - Flags: review?(mak77)
ok, let's see how it's going:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3423ba5d52f
Comment on attachment 8670845 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

Review of attachment 8670845 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/cookie/nsCookiePermission.cpp
@@ +118,2 @@
>      mCookiesLifetimePolicy = val;
> +  }

the only errors reported by the test harness are:
/builds/slave/try-lx-00000000000000000000000/build/src/extensions/cookie/nsCookiePermission.cpp:115:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
/builds/slave/try-lx-00000000000000000000000/build/src/extensions/cookie/nsCookiePermission.cpp:115:41: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]

and indeed you want some static_cast here for the comparisons. val is int32_t but the consts are uint32_t, you likely want to cast the consts to signed, since we control their value.
Attachment #8670845 - Flags: review?(mak77)

Updated

2 years ago
Summary: Forget to delete UI.("ask me every time" in "Keep until") → Remove "Ask me everytime" cookies option.
(Assignee)

Comment 30

2 years ago
Created attachment 8671448 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

Is this fixes the errors ?!
Attachment #8670845 - Attachment is obsolete: true
Attachment #8671448 - Flags: review?(mak77)
Comment on attachment 8671448 [details] [diff] [review]
bug606655_Delete_AskMeEveryTime_UI_Comments_Tests.diff

Review of attachment 8671448 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, it should be good.
Another Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bae47d565151
Attachment #8671448 - Flags: review?(mak77) → review+

Updated

2 years ago
Keywords: checkin-needed

Comment 32

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/ba589252e3f4
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ba589252e3f4
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Successfully reproduce this bug on Firefox 4.0b8pre by following Comment 0 in Linux,64 bit

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b8pre) Gecko/20101023 Firefox/4.0b8pre

This Bug is now verified as fixed on Latest Firefox Nightly 44.0a1 (2015-10-15)

Build ID: 20151015030233
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0
QA Whiteboard: [bugday-20151014]
I have reproduced this bug with Firefox Nightly 4.0b8pre (Build ID: 20101023031633) on 
windows 8.1 64-bit with the instructions from comment 0 .

Verified as fixed with Firefox Nightly 44.0a1 (Build ID: 20151020031317)

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0

As it is also verified on Linux (comment 34) , marking it as verified .
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified

Updated

2 years ago
Duplicate of this bug: 1016077

Updated

2 years ago
Duplicate of this bug: 546746

Updated

2 years ago
Duplicate of this bug: 469260

Comment 39

2 years ago
from https://developer.mozilla.org/en/docs/Cookies_Preferences_in_Mozilla#network.cookie.alwaysAcceptSessionCookies
>network.cookie.alwaysAcceptSessionCookies
>
>Default Value: false
>
>Only used if network.cookie.lifetimePolicy is set to 1
>
>true = accepts session cookies without prompting
>false = prompts for session cookies

Given how this bug was patched, is there a follow-up bug to remove the network.cookie.alwaysAcceptSessionCookies pref altogether, along with a bug to update that documentation?
Flags: needinfo?(mak77)
(In reply to Mardeg from comment #39)
> Given how this bug was patched, is there a follow-up bug to remove the
> network.cookie.alwaysAcceptSessionCookies pref altogether, along with a bug
> to update that documentation?

I think we need such a bug, good idea, actually looks unused. Regarding documentation we can use the keyword.
I'll file a bug for the removal of alwaysAcceptSessionCookies.
Flags: needinfo?(mak77)
Keywords: dev-doc-needed
filed bug 1234875
Blocks: 1235199
Duplicate of this bug: 365772
Duplicate of this bug: 1001399

Comment 44

2 years ago
Very disappointed by cookie setting changes in Firefox 44. I previously commented about this upcoming loss of functionality on https://bugzilla.mozilla.org/show_bug.cgi?id=365772 and was told to move my comment somewhere more visible; I held off until I could see the result. It's even worse than I thought it would be.

I have completely lost the ability to choose which cookies to accept - that was very sad, but expected. What I didn't expect is that the default setting would become Accept! You took control away from me, and then decided that instead of giving me the safe setting (Reject) as a default, you'd just leave me as open as possible to privacy violations.

And it gets even worse. I had a long list of blocked sites in the exception list, mostly pressing the Deny button in the cookie prompting dialog with the "Use my choice for all cookies from this site" checkbox checked. The upgrade to 44 erased the entire exception list! You opened me up not only to new privacy violating cookies but even the ones I had explicitly and permanently rejected before.

It's hard to think of how you could have done a worse job.

Submitted with Firefox 43, updates disabled.

Comment 45

2 years ago
I agree completely with Alan Curry. I usually set the option to "ask me every time" every time I install Firefox.

The option to "Ask me every time" was already buried in a non-intuitive dialog. You had to choose "Firefox will: Use custom settings for history" in Privacy preferences, which then brought up previously hidden options, and then you had to specifically choose "Keep until: Ask me every time" from a drop-down box. So the people who wanted this feature enabled had to actively go looking for it and enable it. It's pretty unlikely that people who enabled this feature were angry at having to click on multiple modal dialog boxes (or, if they were, they blamed the web site authors, not Firefox), since they enabled it intentionally.

Firefox advertises itself as a champion of user privacy. With this change, Firefox arguably reduced user privacy, and made Firefox more like every other mediocre web brower out there.

In (marked duplicate) bug https://bugzilla.mozilla.org/show_bug.cgi?id=469260, in comment #6, Alex Faaborg says (paraphrasing): This change will make some people angry, but that's why we have extensions. 

So where's the extension that replaces this functionality?

Comment 46

2 years ago
Absolutely agree with the above sentiment. I am forced to accept cookies and compromise privacy from all sites now, or have no cookies at all and lose functionality from sites I trust. And if you suggest the "Exceptions" panel can be used to block individual sites I'll point out that it barely works. You can't toggle the block/allow setting, you have to enter each URL individually in the box at the top (and there's no copy URL, hand typing only!) and even if it did work it's cumbersome and disruptive to use constantly.

If this change had been made by Google I would cynically assume that it was to leverage the lack of privacy for monetary gain, and that the nebulous "we don't support that" reason was just bad spin. But in this case I have no idea. The functionality DID work even if the implementation was rough and caused occasional crashes for those who used it. Now we have a browser that crashes less often but sells you out to any new site you visit.
I totally agree with the above comments. Now many websites I browse are broken because of the very limited options I have to manage their cookies, which I block by default.

I did not have any crash while using this option and found it very useful and, moreover, very instructive as to by whom my personal data could be retrieved. Before I started using the "Ask me everytime" option for cookies, I could not know easily which websites were creating side-party cookies.
Then I could see that many of those were ad-servers and that some of them were side-servers, which added functionalities to the main server to make it work.

Bug #606655 was not even a bug, nor a useless UI, nor a useless functionality. I wish it gets reverted in the next release.
Count me in for a full backout of this one. I am using Seamonkey and not Firefox but this "fix" here broke it there too. Thank god I use private builds and was able to back this one and Bug 1234875 out for now. Never saw so many junk cookies in such a short time. Did doubleclick or another "entrepreneur" paid Mozilla to do this?

Comment 49

2 years ago
My favorite Firefox feature was just removed and I am not happy about it.  The "Ask me Every Time" cookies option was the main reason why I used and promoted Firefox.

I am now forced to accept cookies and compromise privacy from all sites now, or have no cookies at all and lose functionality from sites I trust.

Agree 100% with this statement:

"Firefox advertises itself as a champion of user privacy. With this change, Firefox arguably reduced user privacy, and made Firefox more like every other mediocre web brower out there."

Why should I use Firefox now?  Please restore and improve this function!  Might as well use Safari or Edge which provide greater OS integration.  I have put Firefox on over 1,000 systems.  I won't be putting it on another 1,000 systems.  The main selling point is now history.
The functionality was unmaintained, bogus and not really nice to use on today's Web. Yes, it would be better if implemented in an add-on, so someone could take care of it properly and interested users in the community could drive its future better.
Honestly today we have better replacements to protect privacy, you can control third-party cookies and tracking protection can be enabled through privacy.trackingprotection.enabled (in newer versions directly from Options / Privacy), plus there's a bunch of nicely made add-ons that can block any kind of contents on AMO.
Sticking to a dialog that pops up every other second and can easily break website functionality, is just not being realistic, Mozilla is still fighting for users privacy (tracking protection is a clear sign of that), but the tools used to do that should be on par with the current Web and should have the due quality, imo.

Comment 51

2 years ago
Very bad change!
The “ask me every time” was a crucial functionality which is not replaced by any “privacy options”. In fact as wrote:
Charles Evans 2012-03-27 17:25:41 PDT
on
https://bugzilla.mozilla.org/show_bug.cgi?id=469260
remembering history and third party cookies should be managed separately which is not the case.
With respect to the latest comment from Mak:
[…] Sticking to a dialog that pops up every other second and can easily break website functionality, is just not being realistic […]
I don’t see what the problem is. It is a personal choice and nobody is force to such a “micro-cooking-management” if he does not like it.
Rather, I would be curious to know more about how the suppression of the “ask me every time” was managed and which discussion led to the present situation. Somebody knows?
As Alan Curry pointed out before, it is so bad that the Ask-me-every-time changed into Accept instead of Never!

Please just restore and improve the Ask-me-every-time function!

I have a question to Frank-Rainer Grahl, does SeaMonkey offer the ask me every time option?

User
>> The functionality was unmaintained, bogus and not really nice to use on today's Web.

Sorry but this doesn't wash. It worked well for me and as it seems for others in the last 15 years. To call a 100 lines or so of code unmaintained is a joke in my eyes.

>> Sticking to a dialog that pops up every other second and can easily break website 

It's clear that you never used the feature. This would only occur if you frequently open new unknown websites. In this case you simply should not use it. If you have a more or less fixed set of sites you pick the choice and forget it afterwards.

user370560: This needed to be removed from Seamonkey also because the backend was removed in this bug here without any prior warning. I just use private builds were I just put it back in.

Comment 53

2 years ago
The Ask me every time cookies option was one of the main reasons I became a Firefox user.  Most clients are resistant to switching browsers.  They are not computer geeks but everyday computer users.  However, when I showed clients the cookie options that Firefox offered they switched immediately!

Why did they agree to switch to Firefox?

1. They liked being proactive in protecting their privacy because the web is not always trustworthy.

2. Can specify settings for each website.  Some websites you trust, some you don't.

3. They don't wish to delegate privacy controls to some unknown third party.


Mozilla needs to RETHINK this decision!  Mozilla prides themselves on being customer focused.  This is not customer focused behavior.

Comment 54

2 years ago
Four major reasons why I use Firefox:

1.  Adblock plus.
2.  Element hiding helper.
3.  Specify which websites can place cookies on my computer and which cannot.  
4.  Remember website text and size.

Three of the four reasons to use Firefox are now available on competing browsers. Option 3 was the differentiating factor. 

Both Microsoft Edge and Apple Safari feature greater system integration which makes their products easier to use for the average user.  Firefox's advantage was privacy and security.


As Wayne Woods nicely stated: 

"I am now forced to accept cookies and compromise privacy from all sites now, or have no cookies at all and lose functionality from sites I trust."

Comment 55

2 years ago
I went looking for extensions to replace this feature, and Privacy Badger looks like a good start. Advise all the other complaining people to look at it.

It's still immature, and seems to be aiming for wide adoption, not targeted at power users, so it relies more on heuristic magic than individual decision-making. Not the fine-grained control we would like.

At least it shows how much power an extension can exercise over cookies. It gives me hope that implementing a satisfactory cookie policy as an extension would be possible.
@Alan Curry
I have found Cookie Controller ( https://addons.mozilla.org/fr/firefox/addon/cookie-controller/ ), which seems more customizable than Privacy Badger although I have not given it a try yet.

Still, I find it a shame that such a feature for privacy control was completely removed from the bare Firefox 44 (I mean without any extensions).
Please have a look at this just posted blog post about more sistematic ways to protect your privacy without excessive micromanage:
http://sporadicdispatches.blogspot.it/2015/11/better-living-through-tracking.html

Comment 56 suggestions sounds like a good opportunity to evaluate too.

Comment 58

2 years ago
It seems that eventually by fixing this non-bug a REAL BUG has now been created:

forcing the ASK_BEFORE_ACCEPT like ACCEPT_NORMALLY (see comment 19 above by M Hamdy) results in allowing, e.g., google.com to install a cookie even if google.com is set to be blocked in the list of exceptions.

Thanks to Alan to point out Cookie Controller. At my first look I was concerned that it was a rather old add-on, but actually after looking more carefully it has been quite recently updated.

Thanks to Frank-Rainer for his answer about SaeMonkey.

On my side I wish to try if I can work "as before" by simply reversing my previous default choice. Now I do not allow any cookie but I will progressively build up a list of sites that are exceptionally allowed to set cookies.

I have two more comments on this older, but related, bug log:

https://bugzilla.mozilla.org/show_bug.cgi?id=469260

At its beginning the bug log reads:

[…] "ask me every time" [...] exposes far too much detail about the underlying implementation of the Web & It forces the user to make a ridiculous number of decisions […] amazon.com produces 8 dialog boxes, and ebay.com 15

1)	Was indeed to avoid the exposure of far too much detail that triggers the present situation?!
2)	I really want to think the other way around:  it is ridiculous that a web site wants to set 8 or 15 cookies!

As “az_pchelp” wrote: Mozilla needs to RETHINK this decision! Also to fix the bug that has artificially created!

And to answer to the latest comment from Mak:
Using private browsing is not an alternative when one likes to save the browsing history and, moreover, it does not solve the real bug above and the ability to control cookie permission.
I never suggested to use private browsing, please re-read what I suggested.
(In reply to user370560 from comment #58)
> forcing the ASK_BEFORE_ACCEPT like ACCEPT_NORMALLY (see comment 19 above by
> M Hamdy) results in allowing, e.g., google.com to install a cookie even if
> google.com is set to be blocked in the list of exceptions.

please file a separate bug with an example url, I don't think ACCEPT_NORMALLY should ever bypass the exceptions list.

Comment 61

2 years ago
(In reply to Marco Bonardo [::mak] from comment #59)
(In reply to Marco Bonardo [::mak] from comment #60)

I am sorry but then I cannot see what your actual suggestion is, can you please be more explicit?

For what I do understand (and used to do), “tracking protection” and “cookie discrimination or micromanagement)” are two different aspects. I may at some point need to allow a web site to track me in order to receive a certain service, but, maybe, I may still prefer to avoid that that website installs too many cookies. 

Not in a structured way, but I did experience an alike situation with specialise research journals, e.g. elsevier.com or webofknowledge.com Typically the needed service they provide can be achieved by let them install a single cookie.

About the filling of a new bug record, I would do it if it was a new bug. In my experience/opinion this is a consequence of the above. But let see if somebody else confirms.
(In reply to user370560 from comment #61)
> I am sorry but then I cannot see what your actual suggestion is, can you please be more explicit?

I'm saying that, in general, contributing to public blocklists could be more useful to the Web than micromanaging your own list. And could free you up from most of the burden.
Tracking protection (can be enabled in normal browsing mode, not just private browsing) uses the disconnect.me list, either the common or the strict one at your choice.
Other add-ons provide more blocking facilities, out of my mind (there are surely more):
* Ghostery
* Cookie controller
* Privacy Badger
* Ublock (origin or not)
* AdBlock plus

Most of them can also manage a local blocklist.

> About the filling of a new bug record, I would do it if it was a new bug. In
> my experience/opinion this is a consequence of the above.

Thanks, bugs are never fixed when they are not filed :)

Note, I'm not trying to say you are wrong, and I don't think you are. The problem is that it's very hard for a product managed by a relatively small team to satisfy everyone, and sometimes you are that unsatisfied person, I'm sorry, we need to concentrate on the stuff that matters to most users. Luckily Firefox add-ons provide that modularity that is what made it famous in the first place.

That said, I feel like the discussion is going to loop on the same arguments from now on, if you want to discuss the issue further I suggest moving that discussion to firefox-dev or platform-dev mailing lists, since we are abusing a bug tracker and 21 cc-ed persons.

Comment 63

2 years ago
"Your door lock was sticking, so we just removed it entirely. But we care about security because we work heavily on new window locks. Pay no attention that your door lock was removed and look at these shiny window locks!"

Sorry Marco. What you are promoting may be worthwhile, but to do so in context of something else that was removed comes across like a political diversion, like you want the problem to simply go away. People simply don't react well to that.

Comment 64

2 years ago
What animosity towards people wanting to control their own privacy as they see fit.  Telling people that they are wrong to want to control their privacy in a way that makes them comfortable and use "public" block lists.

I want my privacy in a way that I want it.  If I want shear curtains, I put up shear curtains.  If I want to block 8 of 10 cookies from site X, I will block 8 of those 10 cookies.  If I need to allow more to get something from the site, then I will make that choice.

If I don't want to allow the site to track me, I will leave the site.

Even now I have cookies in my settings that I didn't even know that I was getting.  I have now been tracked by an unknown site.

Thanks for disregarding my privacy and the privacy of others by making this change.

As for sites that are broken when they attempt to invade my privacy, it is their problem.  I don't use those sites.

I have already heard of issues at one university over this change as well as putting preferences only in a tab.

Comment 65

2 years ago
Another comment on this feature reduction.  I just wanted to add a site to the block list but with the removal of being able to open preferences in a sepearate window, I cannot have the tab of the site I want to block open and in view at the same time as the preferences so I can type it in.

Firefox is removing the reasons to use it.

Comment 66

2 years ago
Great.  Now I have to block all cookies from all sites until I want to access the site.  Now sites are really broken.

My work productivity has just dropped due to this stupid change.

Comment 67

2 years ago
Just found out, that when I allow cookies, I get Accept third-party cookies: Always by default.

This is bad, very bad for privacy.

I looked at the other options recommended but they take allot more work than "Ask me every time" did to use.

Comment 68

2 years ago
(In reply to Marco Bonardo [::mak] from comment #62)
> That said, I feel like the discussion is going to loop on the same arguments
> from now on, if you want to discuss the issue further I suggest moving that
> discussion to firefox-dev or platform-dev mailing lists, since we are
> abusing a bug tracker and 21 cc-ed persons.

Concur. I'm restricting comments. Please use https://wiki.mozilla.org/Firefox/firefox-dev or the relevant platform newsgroup/mailing lists for further comments, keeping in mind some of the abundant copies of rationale as to why this was done that have already been presented in this bug.
Restrict Comments: true

Comment 69

2 years ago
(In reply to Robin from comment #67)
> Just found out, that when I allow cookies, I get Accept third-party cookies:
> Always by default.

Robin: Confirming that the dropdown value changes. Please file a separate bug report about it.

Updated

2 years ago
Blocks: 1119183

Updated

2 years ago
Depends on: 1249151

Comment 70

a year ago
Isn't this change a contravention of Mozilla core principle #5 on https://www.mozilla.org/en-US/about/manifesto ?

Comment 71

a year ago
(In reply to Scott A. Colcord from comment #70)
> Isn't this change a contravention of Mozilla core principle #5 on
> https://www.mozilla.org/en-US/about/manifesto ?

We think there are sufficient options for you to shape your internet experience within the remaining options of Firefox as well as those offered by add-ons.

Note that this bug's comments are restricted to users with editbugs, and that was done for good reason, considering the general level of debate about this feature removal. So *please* don't reply here.

If you disagree with the decision made here, please read the lengthy thread on firefox-dev: https://groups.google.com/forum/?fromgroups=&hl=en#!topic/firefox-dev/3mnR3ZTGSFU which enumerates several issues with the feature before it was removed, as well as a number of alternative settings and add-ons. You can reply there if you still think that there is something else to be done. I repeat, please don't reply here.

Updated

a year ago
Depends on: 1255149
You need to log in before you can comment on or make changes to this bug.