Closed Bug 716412 Opened 10 years ago Closed 2 years ago
compact dialog prompt: unchecking "Always ask me before compacting folders automatically" fails to change or keep mail
.purge .ask preference to false
31.28 KB, image/jpeg
1.86 KB, patch
|Details | Diff | Splinter Review|
51.72 KB, image/png
62.05 KB, image/png
21.98 KB, application/vnd.oasis.opendocument.text
8.54 KB, patch
|Details | Diff | Splinter Review|
"Always ask me before compacting folders automatically" doesn't always work / stick There are many examples of this in gsfn - the most recent is http://getsatisfaction.com/mozilla_messaging/topics/compct_folder [sic] Some have been collected under the tag http://getsatisfaction.com/mozilla_messaging/tags/compactask - for example http://getsatisfaction.com/mozilla_messaging/topics/please_stop_asking_me_if_i_want_to_compact_folders .... though many failed to disable the ask prompt because of hangs or crashes following the reporter's click of the prompt as in http://getsatisfaction.com/mozilla_messaging/topics/compacting_files -- which is a different bug (fixed I think/hope)
Summary: "Always ask me before compacting folders automatically" doesn't always work / stick to stop the compact dialog prompt → "Always ask me before compacting folders automatically" doesn't always work / stick to stop the compact dialog prompt to change mail.purge.ask preference
The first of your links probably has a typo. But I do not understand what is requested here. Does the dialog not save the preference to not ask again?
(that was a sucky summary) correct. across multiple releases there are cases where user unchecks the setting, click OK, but they continue to get prompted. But I don't have a handy test case for you. ANd when I encounter one in the support forum I have them go into config editor and change the pref. Do you need a failing testcase to troubleshoot this?
Summary: "Always ask me before compacting folders automatically" doesn't always work / stick to stop the compact dialog prompt to change mail.purge.ask preference → compact dialog prompt: unchecking "Always ask me before compacting folders automatically" fails to change or keep mail.purge.ask preference to false
I can't see the problem. The pref is saved for me when TB exits. I don't think there is an OK button. I see a Cancel and Compact button.
I've been brainwashed to think, "OK" button :( two examples - http://getsatisfaction.com/mozilla_messaging/topics/compact_folders_nag - http://getsatisfaction.com/mozilla_messaging/topics/compct_folders [sic] i was missing an "s" earlier
it is unknown what triggers/causes this. possibilities? 1. mail.purge.ask preference "changed" via uncheck and clicking compact, but something after compact caused prefs.js to not change 2. only happens on some type of folder? 3. only happens on startup or some other type of event? 4. can it happen if user kills tbird before compact completes or if compact silently fails? if #1 occurs, pretty sure it doesn't happen for everyone because there are people who report it never gets set.
If I understand Tb's behaviour on mail.purge.ask correctly, Tb does do next only. (1) Read mail.purge.ask setting upon restart only. (2) If mail.purge.ask=true upon restart, (2-1) Show dialog before start of auto-compact. (2-2) When "Do it automatically..." is checked at dialog by user, never shows dialog any more, and stores mail.purge.ask=false in prefs.js. (3) If mail.purge.ask=false upon restart, never shows dialog. So, manual change from mail.purge.ask=false to mail.purge.ask=true won't become effective until restart. And, manual change from mail.purge.ask=true toail.purge.ask=false can do nothing while Tb is running. Checking of "Do it automatically..." at dialog is still needed, until restart of Tb is executed. Wayne, please distinguish (a) manual change of mail.purge.ask=true/false via Config Editor case(as for mail.purge.ask=true/false, it's similar to prefs.js editing while Tb is running), and (b) actual disabling of dialog via dialog case, please. And in (b), check difference between "Compact" case and "Cancel" case, please. When "Cancel", checking of "Do it ..." may also be canceled.
You may be right WADA. I also toggled the pref manually in about:config and got inconsistent results. I'll try again with this info.
FYI. Enabling/disabling of "Always ask me before compacting folders automatically" was orginally done by UI for it, and mail.purge.ask merely holded current status and was read upon restart only. Current Tb's behaviour is simply a result by "removing the UI and only putting disabling part in dialog".
can we crowdsource this issue? Given our numerous compact issues, and the fact that I/we have ignored this issue for a long time, I'd like to see this resolved for TB17. (yeah, that's ambitious - but maybe we can convince Mark to take it) bug 469050 would also be nice, but we've missed the localization boat for TB17. > read upon restart only. Can we inspect the code? If that IS the code then it must be fixed. (hard to tell via UI without bug 469050 :( ) But if it's not read upon restart only - which I think is true - then there is a bug. AFAIK know there are people for whom unchecking "ask" doesn't work. And, a different bug, I think changing the option and then clicking cancel doesn't affect the option. so we should change the code so it does. Or the dialog should somehow to reflect that it doesn't. See screen shot.
I wonder if the sense of checked+"Always ask..." (which seems like negative logic) shouldn't be changed to unchecked + "Don't ask ..." Regarding ask working or not, it seems (I'm not certain) that the frequency is way down for people unchecking the box and reporting that the prompt doesn't go away. Anyway, I'm collecting examples at https://getsatisfaction.com/mozilla_messaging/tags/compactaskfail
Looking at the code putting up the dialog http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgDBFolder.cpp#1883 (this is the only place that uses the mail.purge.ask pref) I do not see any obvious problem. It seems to properly check the pref. 1. if it is true, it shows the dialog 1-1. if the user unchecks the "always ask", it is saved as 'false' but ONLY WHEN THE "Compact" BUTTON IS PRESSED. If Cancel us pressed compact is aborted, but the pref is NOT saved, so it stays at true. 2. if it is false, no dialog is shown, so there is no way to set it to true (only via about:config). I that what you all see?
Wayne's proposal in screenshot can't be done right now, because ConfirmEx() does not allow such fancy design. I've prepared a quick fix for TB17 in that it saves the pref regardless of which button is clicked. The Cancel button is also renamed to No so that is does not imply it is just a plain dismissal of the dialog without any changes saved. This is similar to the solution in bug 476426. For TB18 I'll prepare a better patch with string improvements.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #668911 - Flags: ui-review?(bwinton)
Thanks for the patch. I am somewhat undecided if I like the pref being set if it is unchecked and the user clicks no - because most users frankly don't know what compact is for, and we default to autocompact so that users will get the benefit of having their folders compacted. Put another way, if they select No, then, do they also know that they should be doing manual compacts? Worst case, they uncheck the option, click No, and never compact again. It may or may not lead to performance issues. They can't hit a 4GB barrier any more (depending on OS), which is good. OTOH, always saving/changing the pref is exactly what the UI conveys. So, I like it, with minor reservations. I took another spin through getsatisfaction. Except for the two recent postings , whose meaning is still unclear, I can't find any postings that 100% confirm that the pref isn't being saved when unchecked.  did it not honor unchecking? https://getsatisfaction.com/mozilla_messaging/topics/compact_file_message_turn_off https://getsatisfaction.com/mozilla_messaging/topics/compact_folders_message-13wqn3
Whiteboard: [gs] → [gs][no l10n impact]
Wayne, I am not sure I understand what you say. If you say the default setting is to Compact, but Ask before it. So when the dialog comes up, the Ask is always set. If the user unticks it and it gets saved, then the dialog never comes up again, but compaction will still happen silently automatically in the future. The only difference between the two buttons is that on NO compaction does not happen now, but will happen silently in an hour. On pressing Compact it will be done immediately. If the pref is unset the dialog never comes up so they can never tick it to set it. Only via about:config. Ideally the dialog would have both prefs exposed (Compact and Ask), but that would need a rewrite in XUL and produce a dialog similar to the System integration one ("Set Tb as default client for"). Or having 4 buttons fro all combinations (like "Don't compact now, but ask later", "Don't compact now and never without asking", "Compact now, but ask in the future too", "Compact now and never ask"). So what is the problem?
Or instead of 4 buttons we can properly explain in the dialog what those 2 buttons actually do and that to disable compact you must visit preferences.
I'll get to comment 15 in due course - although I will be very happy to be preempted by others responses. What I want to mention is three things: 1. If we can get rid of the ask part of dialog we might be better off, and if the pref is still wanted then expose it in preferences where we have more room to convey what it does. Which leads me to... 2. I'm not keen about the wording "save disk space", which no doubt most users fixate on thinking this is some sort of option like compression. Indeed... 3. iirc one or more localizations translate "compact" to "compress" Finally, one goal of Bug 286888 is to make this bug obsolete
In the proper patch we can expand the wording to convey what compact really means.
(Can I get a set of steps to reproduce? I'm having a hard time trying to figure out how to test this change. Or, heck, write a Mozmill test, and I can do that manually. ;)
Comment on attachment 668911 [details] [diff] [review] patch for TB17 It does seem weird that "Cancel" would make an action occur. But I see how someone would want to not sync, and not be asked again, and not want another button for that, and there's currently no way to reflect that in the UI. We could add another button, but that's also not great. The ideal solution, I think, to to make the dialog go away entirely, but I hear there a few compact bugs we want to fix first, so in the meantime, ui-r=me, based on the description of the behaviour. (But _please_ add tests, so that the reviewer can make sure this actually does what it says without having to wait an hour for it to trigger on its own… ;) Thanks, Blake.
Attachment #668911 - Flags: ui-review?(bwinton) → ui-review+
You can trigger it just by restarting TB and deleting some big message :) Yes, Cancel should not trigger an action, that is why I rename it to No. But waiting on a test causes a bug to not get fixed :)
And landing patches without tests causes regressions and makes it really hard for other people to fix things in the future. :(
Comment on attachment 668911 [details] [diff] [review] patch for TB17 I realise that the strings suck, but I don't think changing the behaviour of the dialog is a good idea.
wow, I thought we had fixed this. Did we land a different bug else to make the user experience less bad?
I don't know, but the patch here didn't land. So now when we do not need a quick fix for TB17 and probably not TB24, we can fix it properly:) Any suggestions? I'll check if we can get more buttons into that dialog that do the needed actions.
probably a good time to revisit this, in the wake of bug 1157256. Ideally, I'd like to have this for 60.2 release Once we have a fix, we will want explicit testing from users who have recently reported bugs (dupes) and support requests (I think I have a couple), because I am not certain they are all have the same root cause.
So what is the proposal on what to do? My changes were rejected. So what is the proper solution? Also if wanted for 60.2, there can't be string changes again.
(In reply to :aceman from comment #29) > So what is the proposal on what to do? > My changes were rejected. > So what is the proper solution? > Also if wanted for 60.2, there can't be string changes again. If we cannot make progress on the whole package, then perhaps for now we file and fix a new bug that we keep prompting until restart. (In reply to firstname.lastname@example.org from comment #23) > Comment on attachment 668911 [details] [diff] [review] > patch for TB17 > > I realise that the strings suck, but I don't think changing the behaviour of > the dialog is a good idea. What then is the solution to resolving the conflict - because clearly there is a behavior problem in the eyes of the user? Or are you saying the string changes ARE the solution?
In the dialogue box I see, there is a conflict of defacto requirements, as in: In a normal modal dialogue box: the Cancel button is expected to "abandon all". I.E. Close the dialogue and have no other impact. the OK button is supposed to do the promised action according to the configuration displayed. The is a conflict here because the presentation has two actions that are independent. Action 1: Compact the folders Action 2: Change the configuration (I.E. ask/don't ask me again) What some people want is Action 2 but not Action 1, which does not fit the defacto behaviour. A solution picks one of: adding a button, splitting into two dialogues, doing the Action 2 when the Cancel button is clicked, or ... Of course, not making changes is also an option. However, many people have no interest in compacting anymore. Space is cheap.
User Story: (updated)
(In reply to Simon from comment #31) > However, many people have no interest in compacting anymore. Space is cheap. Deleted messages are not gone until compacting occurs. I think there is a greater responsibility to the user to actually remove what they think is gone. While the concept of compaction (perhaps it needs a new name) is about incomprehensible to users and explaining it leaves them thinking about Zip for some reason. They do expect their deleted mail to be gone when they empty the trash. Personally I do not see the point of effort on compaction. I think efforts would be better spent on the mbox replacement. Then compaction can be consigned to the darkness of history.
^ what Matt said. I empty my own trash bin. I even check it first in case I've tossed something recyclable.
I've been digging into the history of compact, and perhaps coming to the conclusion that we should just *remove* this dialog - but not without making a few other changes first. More later about the history and a rationale for removing the dialog, but if someone else has thoughts about removing, or not removing, I'd be interested to hear it.
(In reply to Wayne Mery (:wsmwk) from comment #34) > if someone else has thoughts about removing, or not removing, I'd be interested to hear it. The less said to the user the better. But the habit of the UI to basically lock up while a compact occurs makes silently doing it more difficult, but preferable.
Flags: needinfo?(vseerror) → needinfo?(acelists)
See Also: → 198936
Flags: needinfo?(neil) → needinfo?(alessandro)
Comment on attachment 9067217 [details] Space dialog like so.png It seems clear to me, but of course a native English speaker should give a better feedback. The word `purge` is still used in this screenshot, didn't we want to get rid of it? One quick thing is that the buttons should all be aligned in the same bottom container: ``` [Edit Preferences] [Cancel] [Proceed] ```
Attachment #9067217 - Flags: feedback?(alessandro) → feedback+
Comment on attachment 9067217 [details] Space dialog like so.png Adding Matt for both English and support perspective
Attachment #9067217 - Flags: feedback?(unicorn.consulting)
Comment on attachment 9067294 [details] Space dialog like so v2.png I like the wording, I think it's clear enough. Any reasons why the `Edit Preferences` button was removed. Again, I'd suggest to align the buttons in this way ``` [Edit Preferences] [Cancel] [Proceed] ``` Just to keep it in mind, you don't need to do it in the mock-up, but just as a reminder for when the dialog gets updated.
Attachment #9067294 - Flags: feedback?(alessandro) → feedback+
Comment on attachment 9067294 [details] Space dialog like so v2.png Looks good to me. Just noticed that Outlook calls it "purging recoverable deleted items."
Attachment #9067294 - Flags: feedback?(unicorn.consulting) → feedback+
Comment on attachment 9067217 [details] Space dialog like so.png I do not like the reference to freeing disk space. It was important 20 years ago, it is confusing to general users now to have terabytes free.
Attachment #9067217 - Flags: feedback?(unicorn.consulting) → feedback-
Whiteboard: [gs][no l10n impact] → [gs]
Comment on attachment 9076355 [details] [diff] [review] 716412.patch Review of attachment 9076355 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. ui-r+
Attachment #9076355 - Flags: ui-review?(alessandro) → ui-review+
Comment on attachment 9076355 [details] [diff] [review] 716412.patch Review of attachment 9076355 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Comment on attachment 9076355 [details] [diff] [review] 716412.patch > +autoCompactAllFoldersMsg=It is time for all the messages you have deleted to be removed from disk. This operation will save about %S. Thunderbird can do this automatically without asking you, using the checkbox below. It is Thunderbird for both products. How about &brandShorterName or &brandFullName if you don't want to just display "Daily" in local builds? Would make it generic and SeaMonkey has both strings too. > +autoCompactNeverAskCheckbox=Remove deletions automatically and do not ask me. Both messages sound a little too technical for me. Probably how I would write them myself :) Maybe > +autoCompactAllFoldersMsg=It is time for all the messages you have deleted to be removed from disk. This operation will save about %S. Check the box below to if you want &brandShorterName to do this from now on without asking you. > +autoCompactNeverAskCheckbox=Remove deleted messages automatically and do not ask me again. SeaMonkey is still broken so can't test right now but patch looks otherwise good and jorgk should spot any problems. r+ with at least Thunderbird changed to SeaMonkey in the suite strings.
Attachment #9076355 - Flags: review?(frgrahl) → review+
Comment on attachment 9076355 [details] [diff] [review] 716412.patch The wording is really bad.
Attachment #9076355 - Flags: review?(jorgk) → review-
Comment on attachment 9077202 [details] [diff] [review] 716412.patch v1.1 Review of attachment 9077202 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure why you're presenting the same wording again and expect a different outcome this time. ::: mail/locales/en-US/chrome/messenger/messenger.properties @@ +72,5 @@ > # LOCALIZATION NOTE(compactingDone): %1$S is the compaction gain. > compactingDone=Done compacting (approx. %1$S saved). > autoCompactAllFoldersTitle=Compact Folders > +# LOCALIZATION NOTE(autoCompactAllFoldersMsg): %1$S will be replaced by size gain of the compaction (including the unit), %2$S will be replaced by application name > +autoCompactAllFoldersMsg=It is time for all the messages you have deleted to be removed from disk. This operation will save about %1$S. %2$S can do this automatically without asking you, using the checkbox below. Please change this. "TB can do this automatically without asking you, using the checkbox below." No, TB doesn't do the compaction automatically using a checkbox. Please check the grammar of the sentence. "using" specifies the method by which an action is carried out. He crossed the ocean using a wooden paddle. TB can do this automatically without asking you if you select that option below ... or words to that effect.
Comment on attachment 9077527 [details] [diff] [review] 716412.patch v1.2 Brilliant. I agree that the old message wasn't so helpful, but the new strings have well and truly missed the boat.
Attachment #9077527 - Flags: review?(jorgk) → review+
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Pushed by email@example.com: https://hg.mozilla.org/comm-central/rev/1e23f4af08b6 Follow-up: Reformat. rs=reformat
You need to log in before you can comment on or make changes to this bug.