246738 - Confirm remove all cookies

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, enhancement)

Description

[Joel Craig]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040517 Camino/0.8b
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040517 Camino/0.8b

Twice now, while selectively deleting some cookies in the Cookie Manager, I have
inadvertantly clicked "Remove All", instead of "Remove". A confirmation dialogue
when clicking "Remove All" would prevent these accidents. Also, adding a Check
Box as in Mozilla to "Not Allow Sites That Set Removed Cookies To Set Future
Cookies" would also be a very useful enhancement.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Stuart Morgan]

The confirmation seems like a definite go to me. The check-box is more
debateable; certainly worth considering though.

Comment 2

[Nile]

I think this is a good idea as well. This along with the closing-multiple-tabs
warning are just nice to have.. someone wanna patch rm so 'rm -rf /' has a
confirmation dialogue? :-)

Comment 3

[Josh Aas]

A confirmation is a good idea. I'm not a fan of the checkbox idea.

Comment 4

[Josh Aas]

Attachment: patch v1.0

Also add this to the bottom of Localizable.strings:

/* cookie removal warning */
"Remove All Cookies" = "Remove All Cookies";
"RemoveAllCookiesWarningTitle" = "Are you sure you want to remove all
cookies?";
"RemoveAllCookiesWarning" = "You cannot undo this action.";

Comment 5

[Josh Aas]

Comment on attachment 166436 [details] [diff] [review]
patch v1.0

Not having confirmation is super irritating. Patch is simple.

Comment 6

[Mike Pinkerton (not reading bugmail)]

where's the changes to the localized strings file? I'd like to know what the
message reads before approving.

also,

+      delete mCachedCookies;
+      mCachedCookies = nsnull;
+      mCachedCookies = new nsCOMArray<nsICookie>;

the middle line (setting to nsnull) isn't needed. I know it was present in the
original code, but that doesn't mean we need to carry it over.

Comment 7

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 166436 [details] [diff] [review]
patch v1.0

sr=pink

Comment 8

[Josh Aas]

checked in on trunk with the line pinkerton pointed out removed