"Remove All" button in Privacy: Edit Exceptions sheet has no warning (cookies UI)

RESOLVED FIXED in Camino1.0

Status

--
major
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: alqahira, Assigned: alqahira)

Tracking

({dataloss, fixed1.8})

unspecified
Camino1.0
PowerPC
macOS
dataloss, fixed1.8
Bug Flags:
camino1.0 +

Details

Attachments

(2 attachments, 3 obsolete attachments)

The "Remove All" button in the Privacy pane's "Edit Exceptions List" sheet performs its action without any warning dialogue.  All the rest of our destructive removal UI (including the sheet called by the adjacent "Show Cookies" button in that pane) shows a warning first.

Given the roughness of that UI (bug 314479, bug 314478, bug 303577), this behavior can cause dataloss.
This, of all the cookies UI bugs, most needs to block 1.0 :-)
Flags: camino1.0+

Comment 2

14 years ago
Created attachment 201514 [details] [diff] [review]
Patch for PrivacyPane.mm

This patches PrivacyPane.mm to call NSRunCriticalAlertPanel when "Remove All" is selected. It specifically does not address the issue that more than just the exceptions are removed (bug 314478). Also required is the separately attached Localizable.strings.zip.
Attachment #201514 - Flags: review?(mikepinkerton)

Comment 3

14 years ago
Created attachment 201515 [details]
Localizable.strings file

Replaces mozilla/camino/resources/localized/English.lproj/Localizable.strings
Attachment #201515 - Flags: review?(mikepinkerton)
Comment on attachment 201514 [details] [diff] [review]
Patch for PrivacyPane.mm

Patch looks good. r=me
Attachment #201514 - Flags: review?(mikepinkerton) → review+
Comment on attachment 201515 [details]
Localizable.strings file

Strings changes look good too. r=me, again.

There's no need to zip a strings file though, you can just attach it directly and specify "text/plain; charset=UTF-16" in the future. :)
Attachment #201515 - Flags: review?(mikepinkerton) → review+

Comment 6

14 years ago
I'd like to see the "RemoveAllCookiePermissionsWarning" string be a little more detailed about the consequences (e.g. web site forgetting your identity, losing shopping carts etc).

Comment 7

14 years ago
(In reply to comment #6)
> I'd like to see the "RemoveAllCookiePermissionsWarning" string be a little more
> detailed about the consequences

That particular warning is not part of this bug or this patch. If you meant you'd like to see some more exposition on what happens if you remove all site exceptions, I'll be glad to add that, but I'll need some suggestions. 

As for the warning when removing all cookies, maybe that's best handled as a separate bug report?

Comment 8

14 years ago
(In reply to comment #7)
> (In reply to comment #6)
> > I'd like to see the "RemoveAllCookiePermissionsWarning" string be a little more
> > detailed about the consequences
> 
> That particular warning is not part of this bug or this patch.

Same applies to "RemoveAllCookiePermissionsWarning". Still need to say more than "You cannot undo this action". You need to explain what the consequences are.
Summary: "Remove All" button in Privacy: Edit Ecxeptions sheet has no warning (cookies UI) → "Remove All" button in Privacy: Edit Exceptions sheet has no warning (cookies UI)

Comment 9

14 years ago
(In reply to comment #8)
> Same applies to "RemoveAllCookiePermissionsWarning". Still need to say more
> than "You cannot undo this action". You need to explain what the consequences
> are.

Okay. How about this: "You will be prompted to accept cookies for any web site that attempts to set them. You cannot undo this action."?

I'm also of the opinion that we might consider "This action cannot be undone" in place of "You cannot undo this action." Isn't that the standard way of saying that? But at the same time, I don't want to become inconsistent with the RemoveAllCookiesWarning, so maybe we shouldn't change that part right now. Or maybe we should change them both, or even define a whole new string: "CannotUndoWarning" = "This action cannot be undone." I'm wide open for suggestions. 

Comment 10

14 years ago
Someone remind me: does the exceptions list only apply if you have "ask before accepting each cookie" turned on?
(In reply to comment #10)
> Someone remind me: does the exceptions list only apply if you have "ask before
> accepting each cookie" turned on?

No--if I understand what you're asking.  I deleted my trinity.neooffice.org cookies, changed the Exceptions List entry for trinity to "Deny", unchecked the "Ask before accepting cookies", went to trinity and (attempted to) login.  No trinity cookies when I went to look.

But you can only add items to the Exceptions List (via the Camino GUI) with the "Ask" pref checked.

Comment 12

14 years ago
(In reply to comment #11)
> (In reply to comment #10)
> > Someone remind me: does the exceptions list only apply if you have "ask before
> > accepting each cookie" turned on?
> 
> No--if I understand what you're asking.  I deleted my trinity.neooffice.org
> cookies, changed the Exceptions List entry for trinity to "Deny", unchecked the
> "Ask before accepting cookies", went to trinity and (attempted to) login.  No
> trinity cookies when I went to look.

OK

> But you can only add items to the Exceptions List (via the Camino GUI) with the
> "Ask" pref checked.

It would be nice to allow the user to add sites there by hand.

(In reply to comment #12)
> > But you can only add items to the Exceptions List (via the Camino GUI) with
> > "the Ask" pref checked.
> 
> It would be nice to allow the user to add sites there by hand.

Bug 269084.  

(You can do manual additions already for the Popup Blocker Exceptions sheet--but that only has one state, whitelist, so the UI is easier.)

(In reply to comment #9)

> I'm also of the opinion that we might consider "This action cannot be undone"
> in place of "You cannot undo this action." Isn't that the standard way of
> saying that? 

The existing UI apparently uses both.  Reset Camino is "cannot be undone", Empty Cache is "cannot undo", Clear History doesn't have either one in its message, and in the Prefs, neither Clear Visited Pages nor Clear Cache have an alert at all.  (Clear Visited Pages should--and it should use the same text as Clear History; I'll file a bug on that.)

(The Remove All button in the "Show Cookies..." sheet needs the elipsis, as does this Remove All, once this bug gets wired up.)
Created attachment 202524 [details]
Updated Privacy.nib with elipsis added to Remove All buttons

Here's the nib with the elipsis added to the Remove All buttons.
Created attachment 202619 [details]
proposed Localizable.strings

The Finder/trash says "You cannot undo this action", so I think we should, too.  I've fixed the "Reset Camino" message to use that instead of "cannot be undone" and added that sentence to the "Clear History" string.

Here are the proposals for "RemoveAllCookiesWarning" and "RemoveAllCookiePermissionsWarning"; they're not perfect yet, but starting points (let's build off this localizable.strings file for changes, though, so we keep the cache and history changes all rolled up in one strings change/file):

"RemoveAllCookiesWarning" = "Removing cookies will cause web sites to forget your identity, any online shopping carts, and other personalized site settings. You cannot undo this action.";

"RemoveAllCookiePermissionsWarning" = "Camino will no longer remember which sites you wish to accept cookies from and which sites do not want cookies from.  If you have \"Ask before accepting each cookie\" selected, you will be prompted again to accept cookies for any web site that attempts to set them. You cannot undo this action.";

The second one is more awakward (and, until bug 314478 is fixed, incorrect, too, because it currently blanks hostperm.1 instead of just the cookies entries).
Attachment #201515 - Attachment is obsolete: true
(In reply to comment #16)
> "RemoveAllCookiesWarning" = "Removing cookies will cause web sites to forget
> your identity, any online shopping carts, and other personalized site settings.
> You cannot undo this action.";

"May" rather than "will". Not all these are cookie-based, obviously.

> "RemoveAllCookiePermissionsWarning" = "Camino will no longer remember which
> sites you wish to accept cookies from and which sites do not want cookies from.
>  If you have \"Ask before accepting each cookie\" selected, you will be
> prompted again to accept cookies for any web site that attempts to set them.
> You cannot undo this action.";

How about (for the first sentence):

"Camino will no longer remember which sites you do or do not want to accept cookies from."

I'm not exactly thrilled with that, either, but it's shorter and a little less awkward.

cl

Comment 18

14 years ago
> "Camino will no longer remember which sites you do or do not want to accept
> cookies from."

Or perhaps
"Camino will no longer remember which sites you chose to accept or reject cookies for"?

Curly quotes in the Localizable.strings, please.
--> Smokey.

cl
Assignee: mikepinkerton → alqahira

Comment 20

14 years ago
Fixed.
Keywords: fixed1.8

Updated

14 years ago
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Created attachment 202973 [details]
Updated Localizable.strings

Simon, I didn't see a Localizable.strings checkin associated with any of these bugs in bonsai (only the .strings files that live within the prefPanes).  Here's the updated file with all changes mentioned in this bug and bug 315886.
Attachment #202619 - Attachment is obsolete: true
Attachment #202973 - Flags: superreview?(sfraser_bugs)

Comment 22

14 years ago
Smokey: each prefs bundle has its own Localizable.strings, which is where the strings should be for that pref bundle. We shouldn't use NSLocalizedString() in the pref pane code.
Attachment #202973 - Attachment is obsolete: true
Attachment #202973 - Flags: superreview?(sfraser_bugs)
You need to log in before you can comment on or make changes to this bug.