75119 - Cookie Manager: "Don't allow sites that set removed cookies to set future cookies" should stay checked/unchecked

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[Dennis Andersen]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows 95)
BuildID:    2001040404

The state of the checkbox "Don't allow removed cookies to be accepted later" in 
the cookie permission dialog should be saved

Reproducible: Always
Steps to Reproduce:
1. Go to "Preferences => Advanced => Cookies => View stored cookies"
2. Click "Don't allow removed cookies to be accepted later"
3. Remove some cookies
4. Exit Mozilla
5. Go to stored cookies again.

Actual Results:  The state (checked or unchecked) of the checkbox is not saved.

Expected Results:  The state should be saved.

Comment 1

[Chris McAfee]

over to morse

Comment 2

[sairuh (rarely reading bugmail)]

->cookies.

Comment 3

[tironsi]

What's the status on this bug? It doesn't seem like something that would be all
that difficult to implement.

Oh, and it should probably be changed to all OS's

Comment 4

[benc]

-> default.
clarified summary w/ current UI string

Comment 5

[Darin Fisher]

dwitte, mvl: either of you feel like whipping up a patch here?

Comment 6

[[e-mail]]

i'm sure mvl can do this in about 30 seconds. :)

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

fix would be probably to add a persist="checked" attribute to the <checkbox>

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: add persist

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

The whole concept seems to be implemented in a whacky way. For some reason I
have a really hard time removing cookies so that the site can not set them
later. I think the current implementation requires you to: 1) check the box, 2)
remove cookie, 3) ok the dialog and 4) ok the prefs dialog. I often try to
remove some cookies permanently and some cookies "temporarily", and check the
blocked cookies tab to see if my actions were recorded, and usually they are not.

I suggest that as soon as you hit the remove button while the checkbox is
checked, that site gets entered into the second tab for blocked sites. It is
really counter intuitive to wait for the whole dialog to be dismissed, and not
being able to remove some cookies permanently and some cookies "temporarily" at
the same time.

Comment 10

[[e-mail]]

totally agree - the cookiemanager UI as it stands has a ton of RFE's. we need to
find someone brave enough to hack (or better, rewrite) it ;)

Comment 11

[benc]

Maybe we need to differentiate between just removing a cookie and condeming a
site (and ridding yourself of all the cookies from their site).

Comment 12

[Francesco Castellana]

I can confirm this on Firebird, too.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030724 Mozilla
Firebird/0.6

Comment 13

[Michiel van Leeuwen (email: mvl+moz@)]

*** Bug 222133 has been marked as a duplicate of this bug. ***

Comment 14

[Alec Flett]

Comment on attachment 122883 [details] [diff] [review]
add persist

sr=alecf

Comment 15

[Michiel van Leeuwen (email: mvl+moz@)]

Checked in.

Comment 16

[Steffen Wilberg]

Attachment: do the same for firebird

Comment 17

[Steffen Wilberg]

I guess we don't need a separate Firebird bug for this one-liner.
->Reopening.
Pierre, review?

Comment 18

[Pierre Chanial]

Comment on attachment 135235 [details] [diff] [review]
do the same for firebird

That checkbox should be controlled by a pref and be integrated with the widget
manager mechanism to store it.

Comment 19

[[e-mail]]

could you explain why?

Comment 20

[Pierre Chanial]

The only access point to the cookie viewer is, in Firebird, as a subdialog
called from the preference window. In Firebird, if you remove all the cookies,
then press OK in the subdialog and then Cancel in the main pref window, the
cookies won't be deleted. Consequently, pressing cancel in this subdialog or in
the main pref window should undo a change in that checkbox.

Concerning the suite, I don't think neither that's a good idea to store this
checkbox value in the localstore instead of pref.js
 - if you change the check box state, pressing cancel won't undo the change.
 - deleting the local store, imo should only alter how UI is displayed (window
size, button/window position etc.) and not revert such user decisions.

Comment 21

[Michiel van Leeuwen (email: mvl+moz@)]

In the suite, there is no cancel button. (which might be a bug)
The checkbox is no use decision that has any effect after closing the dialog. It
just affects what happens when you delete a cookie. The persist is just there
because it is likely that a user whats the same option again the next time the
manager is used, just like it is likely he wants it to be the same size.

Becasue the checkbox has no meaning outside the manager window, a pref seems a
bit too much to me.

And over to firebird, as the seamonkey part is fixed.

Comment 22

[Steffen Wilberg]

Attachment: use a pref

Right, I introduced the pref "browser.cookie.blockRemovedCookies" for the
checkbox. I thought of "network" instead of "browser", but then this pref
influences the UI and not the network code. But I placed the pref below the
other cookie prefs in all.js.

Comment 23

[Steffen Wilberg]

Comment on attachment 135312 [details] [diff] [review]
use a pref

Pierre, this is the first time I fiddled around with prefs. But it works.

Comment 24

[Steffen Wilberg]

taking.

Comment 25

[Steffen Wilberg]

Targetting for 0.8.

Comment 26

[Steffen Wilberg]

Comment on attachment 135312 [details] [diff] [review]
use a pref

Moving review request to Ben.

Comment 27

[Ben Goodger (use ben at mozilla dot org for email)]

I don't think this is quite right... the panel is using a GetFields/SetFields
method to manage its prefs... which means the checkbox is getting initialized in
those methods... and something isn't working properly there. 

Kicking to 0.9

Comment 28

[Steffen Wilberg]

Comment on attachment 135312 [details] [diff] [review]
use a pref

I have still no idea what's going wrong in SetFields. We could of course simply
do what SeaMonkey does, i.e. use persist="checked", see attachment 135235 [details] [diff] [review].

Comment 29

[Steffen Wilberg]

Comment on attachment 135235 [details] [diff] [review]
do the same for firebird

Maybe not that bad?

Comment 30

[Mike Connor [:mconnor]]

remind me to look at this at some point

Comment 31

[Blake Ross]

-> me

Comment 32

[Marcia Knous [:marcia]]

For reference, here is testrunner test case that corresponds to this pref.
http://testrunner.mozilla.org/tr_testcaseform.cgi?&case_id=1035&product_id=21

Comment 33

[Blake Ross]

-> Mike

Comment 34

[Steffen Wilberg]

Bug 274712 introduces a network.cookie.denyRemovedCookies pref.

Comment 35

[Mike Connor [:mconnor]]

renominate if the new prefs dialog doesn't change this.  Nominating anything in
the prefs dialog is pretty much useless until the new one lands, since its a
totally different API.

Comment 36

[Steffen Wilberg]

The checkbox is gone. --> kind of fixed.

Comment 37

[Tracy Walker [:tracy]]

The new pref is:
[x]Allow sites to set cookies
  [x]unless I have removed cookies set by the site.

which appears to work with fx trunk build Win 2005-05-18-07-trunk

Comment 38

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change