compact dialog prompt: unchecking "Always ask me before compacting folders automatically" fails to change or keep mail.purge.ask preference to false

ASSIGNED
Assigned to

Status

MailNews Core
Backend
ASSIGNED
6 years ago
3 years ago

People

(Reporter: wsmwk, Assigned: aceman)

Tracking

({ux-control})

Trunk
ux-control

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gs][no l10n impact], URL)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
"Always ask me before compacting folders automatically" doesn't always work / skick

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)
(Reporter)

Comment 1

6 years ago
mail.purge.ask preference
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
Whiteboard: [gs]
(Assignee)

Comment 2

6 years ago
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?
(Reporter)

Comment 3

6 years ago
(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
(Assignee)

Comment 4

6 years ago
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.
(Reporter)

Comment 5

6 years ago
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
(Reporter)

Comment 6

6 years ago
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.
(Assignee)

Comment 8

6 years ago
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".
(Reporter)

Comment 10

5 years ago
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.
Keywords: ux-control
(Reporter)

Comment 11

5 years ago
Created attachment 668889 [details]
compact ask dialog

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
(Assignee)

Comment 12

5 years ago
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?
(Assignee)

Comment 13

5 years ago
Created attachment 668911 [details] [diff] [review]
patch for TB17

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)
(Assignee)

Updated

5 years ago
OS: Windows Vista → All
Hardware: x86 → All
(Reporter)

Comment 14

5 years ago
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 [1], whose meaning is still unclear, I can't find any postings that 100% confirm that the pref isn't being saved when unchecked.

[1] 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]
(Assignee)

Comment 15

5 years ago
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?
(Assignee)

Comment 16

5 years ago
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.
(Reporter)

Comment 17

5 years ago
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
(Assignee)

Comment 18

5 years ago
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. ;)
Flags: needinfo?(acelists)
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+
(Assignee)

Comment 21

5 years ago
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 :)
Flags: needinfo?(acelists)
(Assignee)

Updated

5 years ago
Attachment #668911 - Flags: review?(neil)
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.
Attachment #668911 - Flags: review?(neil)
(Reporter)

Comment 24

4 years ago
wow, I thought we had fixed this. Did we land a different bug else to make the user experience less bad?
Flags: needinfo?(acelists)
(Assignee)

Comment 25

4 years ago
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.
Flags: needinfo?(acelists)
You need to log in before you can comment on or make changes to this bug.