175359 - Add confirm dialog for Remove All Cookies/Remove All Sites

Status: VERIFIED FIXED

(Core :: Networking: Cookies, enhancement, P3)

Description

[Joel Aufrecht]

Both the Stored Cookies and Cookie Sites tabs in the Cookie Manager dialog box
work the same way, and have the same problem: The Remove [one] button is
adjacent to the Remove [All] button, and there's no safety for the Remove all. 
This bit me on the Cookie site tab but I guess it could be an issue on the
Stored Cookie tab as well.  I tend to block all cookies and then go back and
allow specific sites.  If I slip and hit the Remove All Sites button instead of
Remove Site, I lose months and months of accumulated information about which
sites can and can't set cookies.  A confirmation dialog would solve the problem
of mis-clicks.

Comment 1

[Tom Everingham]

sounds like a good idea to me, Steve what do you think?  

Comment 2

[Darin Fisher]

yes, a good idea -> suresh

Comment 3

[Suresh]

Attachment: patch

Comment 4

[Suresh]

Jatin, can you please review the text/string used in this patch?

+deleteAllCookies=Are you sure you want to delete all the cookies?
+deleteAllCookiesSites=Are you sure you want to delete all the cookies sites?

Thanks!

Comment 5

[Jatin Billimoria]

Looks good to me, r=jatin

Comment 6

[Alexander Grimalovsky]

Since a patch and review for it is available for 9 months already - isn't it a
good idea to include it into nearest Mozilla milestone?

Comment 7

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

Comment on attachment 113733 [details] [diff] [review]
patch

> function DeleteAllCookies() {
>+  if (!confirm(cookieBundle.getString("deleteAllCookies")))
>+    return;

Use the promptservice instead of confirm().

Comment 8

[Mike Connor [:mconnor]]

-> me

Comment 9

[Mike Connor [:mconnor]]

Attachment: patch against tip using promptservice.

this also differentiates the dialog for Remove All Sites depending on whether
it is for images or cookies, which the original did not do.

Comment 10

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

Comment on attachment 138504 [details] [diff] [review]
patch against tip using promptservice.

>Index: mozilla/extensions/wallet/cookieviewer/resources/content/CookieViewer.js

>@@ -532,14 +540,20 @@ function buttonEnabling(textfield) {

>+  var msgType = dialogType == cookieType ? "deleteAllCookiesSites" : "deleteAllImagesSites";

I personally find it more clear to write | msgType = (dialogType == cookieType)
? |

>Index: mozilla/extensions/wallet/cookieviewer/resources/locale/en-US/CookieViewer.properties

>+deleteAllCookies=Are you sure you want to delete all the cookies?
>+deleteAllCookiesTitle=Remove All Cookies
>+deleteAllCookiesSites=Are you sure you want to delete all of the cookie sites?
>+deleteAllImagesSites=Are you sure you want to delete all of the image sites?
>+deleteAllSitesTitle=Remove All Sites

r=mvl, but i'm not sure i like the "delete cookie sites" part. I would like to
delete doubleclick.net, but i can't ;) But maybe it is just me not being native
english. Let someone else look at that.

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 138504 [details] [diff] [review]
patch against tip using promptservice.

I guess my feeling on the cookie sites bit is that its not perfect, but that's 
what the tab is called.

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 138504 [details] [diff] [review]
patch against tip using promptservice.

moving this to alecf, darin is pretty swamped for SR

Comment 13

[Alec Flett]

Comment on attachment 138504 [details] [diff] [review]
patch against tip using promptservice.

nice. sr=alecf

Comment 14

[Mike Connor [:mconnor]]

landed on trunk 01/27/2004 08:41

Comment 15

[benc]

V/fixed:
Mac OS X, 1.7.2.