Closed Bug 1402558 Opened 3 years ago Closed 3 years ago

Unexpected changes to whether email bodies stored locally for imap account folders

Categories

(MailNews Core :: Account Manager, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 59.0

People

(Reporter: gds, Assigned: gds)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170814061550

Steps to reproduce:

I have setup an imap account and have kept the default Synchronization and Storage setting of "Keep messages for this account on this computer" checked. I have gone into the "Advanced..." setting below this and have selected several folders that I actually don't want stored locally because they are so big they would exceed my disk capacity. I run with this setup for several month or even years and, for no real reason, revisit the Synchronization and Storage screen. I then accidentally uncheck the "Keep messages for this account on this computer" checkbox.  I notice what I did and check it back again and to make sure I didn't change anything I click Cancel at the bottom of the Account Setting screen. I assume no harm is done since I set the checkbox back and I clicked cancel.


Actual results:

After some time I start noticing that my disk is almost out of space. Looking in my profile under ...imapmail/<accountX>/ I see large files that account for the excessive disk usage. These files contain email bodies from the folders that, years/months ago, I thought I had excluded from local storage. I check the Synchronization and Storage screen for this account and see that the checkbox for storing emails locally is still set and I notice the "Advanced..." button right below it. I click the advanced button and see that *all* the folders are selected for offline use, which just means email bodies are stored locally for all folders. I then vaguely remember from years ago that I excluded some folders with this screen. So why are they now *all* included again and filling up my drive? Even junk and trash are being stored locally which I'm sure I would never intentionally do.


Expected results:

The problem was caused when I accidentally unchecked the "Keep messages for the account on this computer", checked it back and then canceled out of the account settings. What actually happens is that when "Keep messages ... on this computer" is unchecked, all folders for this account are immediately set to "don't keep messages for this folder on this computer" and only store the headers locally. And when "Keep messages ... on this computer" is re-checked, *all* folders for the account are immediately set to store messages locally, including trash and junk. Clicking cancel after clicking on "Keep messages ... on this computer" has no effect, unlike other items in Account Settings screens. Unlike other checkboxes and radio buttons in the account settings screen, the "Keep messages ... on this computer" checkbox has an instant and (almost) irrevocable effect.

Ideally, clicking cancel at this point should cause any resulting settings to be reverted but it doesn't when you click "Keep messages for this account on this computer".

If "Keep messages ... on this computer" checkbox is changed, the user must know to go into the "Advanced..." screen and re-select the various folders for the account offline or not-offline use (i.e., store bodies locally or store only headers).  Note: Within the Advanced screen the cancel button has the expected effect and reverts any changes.
Account management: A case for Aceman, unless you want to take the bug.

I've vaguely followed bug 1399380, but I must admit, I've only visited the IMAP sync settings in the account settings today for the first time :-(
Flags: needinfo?(acelists)
Component: Untriaged → Account Manager
Been trying to fix. Found fn onCancel that looks like should do job but never called, per my added dump call.
https://bug635044.bugzilla.mozilla.org/show_bug.cgi?id=436615#c94
Seems like 9 years ago someone noticed that onCancel() was never called ("bad news") and that the "Keep messages on this computer" checkbox has an instant effect ("Eww"). However, I see no response to this comment in bug 436615 or in the associated patches or in the current tb code.

(In reply to Jorg K (GMT+2) from comment #1)
> Account management: A case for Aceman, unless you want to take the bug.

Trying to fix but no luck so far. Will let you know when I give up.

> 
> I've vaguely followed bug 1399380, but I must admit, I've only visited the
> IMAP sync settings in the account settings today for the first time :-(

I've had some dealing with it not in depth until bug 1399380.
Attached patch 1402558-review-changes0.patch (obsolete) — Splinter Review
Well, this almost fixed it. But it only works if Cancel occurs when you remain on the "Synchronization and Storage" screen. If you move to another screen in the account or to a different account and click Cancel the onCancel() function is never called. Also, for some screens such as Junk Settings, if you click Cancel there after a few seconds you end up with a "recursion limit" error on the top level onCancel() and no cancel occurs. Don't yet know why.
In the patch, if the new function renamed from onCancel to onNotAccept it fixes the recursion. But other problems still present and still trying to fix. No updated patch yet.
Attached patch 1402558-review-changes1.patch (obsolete) — Splinter Review
This fixes the problem. However it requires string changes and a new prompt. Without string changes and the new prompt, you can only cancel the sync changes if you click cancel while still on the Sync and Storage screen. As soon as you move from Sync to another config page for the account or to another account, the sync changes are effectively automatically saved. So to solve this, when moving to a new page, but before the Sync page is exited, I put up a prompt to allow the user to cancel or save any sync changes made. The prompt only appears if a sync change was actually made on the Sync/Storage/Advanced screen.
A possible improvement might be to provide a 3 button prompt so you can also remain on the Sync & Storage and do further edits there instead of moving to the new page.
Attachment #8911670 - Attachment is obsolete: true
Comment on attachment 8913061 [details] [diff] [review]
1402558-review-changes1.patch

Please request review or feedback when you think that it's in a presentable state.

There is nothing wrong with string changes, only that we can't backport them.
Attachment #8913061 - Flags: feedback?(acelists)
Comment on attachment 8913061 [details] [diff] [review]
1402558-review-changes1.patch

Forgot to mention that I also modified the string for the checkbox "Keep messages for this account on this computer" so it better describes what checking/unchecking this actually does.
Also, as practice I requested you (Jorg) for feedback. Sorry, I always forget this and never sure how to do it. But I think I see now.
Attachment #8913061 - Flags: feedback?(jorgk)
Comment on attachment 8913061 [details] [diff] [review]
1402558-review-changes1.patch

I think Aceman is the better man here. He's the account manager specialist (and I'm 200% busy with other stuff).
Attachment #8913061 - Flags: feedback?(jorgk)
Attachment #8913061 - Flags: feedback?(acelists)
Comment on attachment 8913061 [details] [diff] [review]
1402558-review-changes1.patch

Review of attachment 8913061 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this works much more robust and alerts the user changed did happen.

I would be willing to land this without much rearchitecting because we already have such dialog when you switch pages after server username so your approach is not new in AM. I can look at rearchitecting this in a new bug.

::: mail/locales/en-US/chrome/messenger/am-offline.dtd
@@ +5,5 @@
>  <!ENTITY doNotDownloadPop3Movemail.label "To save disk space, do not download:">
>  <!ENTITY doNotDownloadNntp.label "To save disk space, do not download for offline use:">
>  <!ENTITY doNotDownloadImap.label "To save disk space, downloading messages from the server and keeping local copies for offline use can be restricted by age or size.">
> +<!ENTITY allFoldersOffline.label "Keep messages in all folders for this account on this computer.
> +Note: Changing this affects all folders in the account. To set individual folders, click the Advanced… button.">

Could you move this "Note:" block a s <description> element above or below the checkbox?

::: mailnews/base/prefs/content/am-offline.js
@@ +10,5 @@
>  var gImapIncomingServer;
>  var gPref = null;
>  var gLockedPref = null;
>  var gOfflineMap = null; // map of folder URLs to offline flags
> +var gOffline;           // initial state of allFoldersOfflien checkbox

Offline? But the checkbox isn't named exactly like this. Please make it match. The id is "offline.folders".

@@ +206,5 @@
>      restoreOfflineFolders(gOfflineMap);
>      return true;
>  }
>  
> +function promptSaveSyncChanges()

Can you name this more universal so that more pref page can use this?
It seems some code in AccountManager.js (like checkUserServerChanges()) should then be moved into this function of the respective page.

Maybe the name could be onLeave().

On the other hand, I see what you want to say here. When leaving the Sync page for another one, the onCancel() and restoreOfflineFolders() functions are lost and can't be run when Canceling the AM from a different page.

I wonder if it would be possible to "pack" the code and push it to some global array in AccountManager.js where each member is run on Cancel of the AM? But that may be too complicated (saving all the state to revert at some very late time, where we also may enter the same Sync page multiple times).

@@ +232,5 @@
> +    let question = prefBundle.getString("confirmSyncChanges");
> +    if (!Services.prompt.confirm(window, title, question)) {
> +      // User clicked Cancel, so restore all the sync related changes.
> +      restoreOfflineFolders(gOfflineMap);
> +      document.getElementById("offline.folders").checked =  gOffline;

Surplus space after = .
Attachment #8913061 - Flags: feedback?(acelists) → feedback+
(In reply to :aceman from comment #10)
> Comment on attachment 8913061 [details] [diff] [review]
> 1402558-review-changes1.patch
> 
> Review of attachment 8913061 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks, this works much more robust and alerts the user changed did happen.
> 
> I would be willing to land this without much rearchitecting because we
> already have such dialog when you switch pages after server username so your
> approach is not new in AM. I can look at rearchitecting this in a new bug.
> 
> ::: mail/locales/en-US/chrome/messenger/am-offline.dtd
> @@ +5,5 @@
> >  <!ENTITY doNotDownloadPop3Movemail.label "To save disk space, do not download:">
> >  <!ENTITY doNotDownloadNntp.label "To save disk space, do not download for offline use:">
> >  <!ENTITY doNotDownloadImap.label "To save disk space, downloading messages from the server and keeping local copies for offline use can be restricted by age or size.">
> > +<!ENTITY allFoldersOffline.label "Keep messages in all folders for this account on this computer.
> > +Note: Changing this affects all folders in the account. To set individual folders, click the Advanced… button.">
> 
> Could you move this "Note:" block a s <description> element above or below
> the checkbox?

Moved it to below the checkbox. Also, changed the note slightly:
+<!ENTITY allFoldersOfflineNote.label "Note: Changing this affects all folders in this account. To set individual folders, select the Advanced… button.">

> 
> ::: mailnews/base/prefs/content/am-offline.js
> @@ +10,5 @@
> >  var gImapIncomingServer;
> >  var gPref = null;
> >  var gLockedPref = null;
> >  var gOfflineMap = null; // map of folder URLs to offline flags
> > +var gOffline;           // initial state of allFoldersOfflien checkbox
> 
> Offline? But the checkbox isn't named exactly like this. Please make it
> match. The id is "offline.folders".

Ok, I changed it to gOfflineFolders.

> 
> @@ +206,5 @@
> >      restoreOfflineFolders(gOfflineMap);
> >      return true;
> >  }
> >  
> > +function promptSaveSyncChanges()
> 
> Can you name this more universal so that more pref page can use this?
> It seems some code in AccountManager.js (like checkUserServerChanges())
> should then be moved into this function of the respective page.
> 
> Maybe the name could be onLeave().
> 
> On the other hand, I see what you want to say here. When leaving the Sync
> page for another one, the onCancel() and restoreOfflineFolders() functions
> are lost and can't be run when Canceling the AM from a different page.
> 
> I wonder if it would be possible to "pack" the code and push it to some
> global array in AccountManager.js where each member is run on Cancel of the
> AM? But that may be too complicated (saving all the state to revert at some
> very late time, where we also may enter the same Sync page multiple times).

Not sure are saying to change the name or not? Anyhow, for now, I have left it the same.

> 
> @@ +232,5 @@
> > +    let question = prefBundle.getString("confirmSyncChanges");
> > +    if (!Services.prompt.confirm(window, title, question)) {
> > +      // User clicked Cancel, so restore all the sync related changes.
> > +      restoreOfflineFolders(gOfflineMap);
> > +      document.getElementById("offline.folders").checked =  gOffline;
> 
> Surplus space after = .

Space removed.

I also removed the personal dump("gds: ...) I added for tracing. 
Will attach an updated patch.
Attached patch 1402558-review-changes2.patch (obsolete) — Splinter Review
Attachment #8913061 - Attachment is obsolete: true
Attachment #8936704 - Flags: review?(acelists)
Attachment #8936704 - Flags: feedback?(jorgk)
Comment on attachment 8936704 [details] [diff] [review]
1402558-review-changes2.patch

Review of attachment 8936704 [details] [diff] [review]:
-----------------------------------------------------------------

Looks OK to me. A few nits. I haven't tried is since Aceman is the account specialist and he's got the review well under control. Nice that this annoying bug will finally be addressed. So many messages have been downloaded inadvertently due to it :-(

::: mail/locales/en-US/chrome/messenger/am-offline.dtd
@@ +4,5 @@
>  
>  <!ENTITY doNotDownloadPop3Movemail.label "To save disk space, do not download:">
>  <!ENTITY doNotDownloadNntp.label "To save disk space, do not download for offline use:">
>  <!ENTITY doNotDownloadImap.label "To save disk space, downloading messages from the server and keeping local copies for offline use can be restricted by age or size.">
> +<!ENTITY allFoldersOffline.label "Keep messages in all folders for this account on this computer.">

If you change the string, you need to change the string ID as well, so this becomes allFoldersOffline2.

::: mailnews/base/prefs/content/am-offline.js
@@ +212,5 @@
> +    let allFolders = gIncomingServer.rootFolder.descendants;
> +    for (let folder of fixIterator(allFolders, Components.interfaces.nsIMsgFolder)) {
> +      if (gOfflineMap[folder.folderURL] !=
> +          folder.getFlag(Components.interfaces.nsMsgFolderFlags.Offline)) {
> +        // A change to the Offline flag to a folder was made

Nit. Full stop.

@@ +218,5 @@
> +        break;
> +      }
> +    }
> +  } else {
> +    // A change to the allFoldersOffline checkbox was made

ditto.
Attachment #8936704 - Flags: feedback?(jorgk) → feedback+
Comment on attachment 8936704 [details] [diff] [review]
1402558-review-changes2.patch

Review of attachment 8936704 [details] [diff] [review]:
-----------------------------------------------------------------

We also need a UI review here, I'll ask Paenglab.

::: mailnews/base/prefs/content/am-offline.js
@@ +204,5 @@
>      restoreOfflineFolders(gOfflineMap);
>      return true;
>  }
>  
> +function promptSaveSyncChanges()

Yes I wanted to change the name. It does prompt for sync changes in this prefs page, but will do something else in other pages (if I migrate some code to the function). So what it actually does is run some code when the current page is being left. So I propose name 'onLeave' that is similar to the other functions like onPreInit, onInit, onCancel.

@@ +213,5 @@
> +    for (let folder of fixIterator(allFolders, Components.interfaces.nsIMsgFolder)) {
> +      if (gOfflineMap[folder.folderURL] !=
> +          folder.getFlag(Components.interfaces.nsMsgFolderFlags.Offline)) {
> +        // A change to the Offline flag to a folder was made
> +        changed = true;

Do you want to show the prompt in all cases, or only when the user didn't visit the Advanced sub dialog and changes happened (due to touching the global checkbox)?
Attachment #8936704 - Flags: ui-review?(richard.marti)
Attachment #8936704 - Flags: feedback?(jorgk)
Attachment #8936704 - Flags: feedback+
Comment on attachment 8936704 [details] [diff] [review]
1402558-review-changes2.patch

Why f? again :-(
Attachment #8936704 - Flags: feedback?(jorgk) → feedback+
(In reply to :aceman from comment #14)
> Comment on attachment 8936704 [details] [diff] [review]
> 1402558-review-changes2.patch
> 
> > +function promptSaveSyncChanges()
> 
> Yes I wanted to change the name. It does prompt for sync changes in this
> prefs page, but will do something else in other pages (if I migrate some
> code to the function). So what it actually does is run some code when the
> current page is being left. So I propose name 'onLeave' that is similar to
> the other functions like onPreInit, onInit, onCancel.

Ok, I will change it.

> 
> @@ +213,5 @@
> > +    for (let folder of fixIterator(allFolders, Components.interfaces.nsIMsgFolder)) {
> > +      if (gOfflineMap[folder.folderURL] !=
> > +          folder.getFlag(Components.interfaces.nsMsgFolderFlags.Offline)) {
> > +        // A change to the Offline flag to a folder was made
> > +        changed = true;
> 
> Do you want to show the prompt in all cases, or only when the user didn't
> visit the Advanced sub dialog and changes happened (due to touching the
> global checkbox)?

I would like for cancel to revert any change you make on the sync page, not just if you toggle the "global" box. However, the cancel won't revert any sync changes you make to a different account in Advanced. Anyhow, here's how it now works:

The way the patch works is if the "global checkbox" (actually it just affects all folders of the currently selected account) is *not* changed, then each folder in the account is examined for a change (that was made in the Advanced screen) and if any **in the current account** have changed, the prompt appears when the user selects another item in the categories on the left on the Account Setting screen.
 
If the "global checkbox" is changed, no need to check the individual folders for change so again the prompt appears when the user selects another item in the categories on the left on the Account Setting screen.

If the user changes the "global checkbox" or changes the status of any folder **in the current account** inside the Advanced screen and, immediately after dismissing the Advanced screen, selects Cancel at the bottom of the "Synchronization & Storage" screen, then all changes to online/offline status for the current account are reverted silently.

?? (Maybe the prompt should also occur if there is a sync change and the user selects OK at the bottom of the Sync & Storage window. Maybe only if a "global" box change occurred and not just the user change something in Advanced.)

Note: In the Advanced screen, if the user changes the status of a folder of an account that is *not* in the currently selected account, the prompt to revert provided by the patch does not occur. Also, Cancel at the bottom of the "Synchronization & Storage" screen does not revert this online/offline change in a not currently selected account. This is because the Advanced screen allows access to all imap accounts.
Comment on attachment 8936704 [details] [diff] [review]
1402558-review-changes2.patch

Review of attachment 8936704 [details] [diff] [review]:
-----------------------------------------------------------------

You are changing things in mailnews. This affects SM too and you need to add also the locale changes for suite.

::: mail/locales/en-US/chrome/messenger/am-offline.dtd
@@ +5,5 @@
>  <!ENTITY doNotDownloadPop3Movemail.label "To save disk space, do not download:">
>  <!ENTITY doNotDownloadNntp.label "To save disk space, do not download for offline use:">
>  <!ENTITY doNotDownloadImap.label "To save disk space, downloading messages from the server and keeping local copies for offline use can be restricted by age or size.">
> +<!ENTITY allFoldersOffline.label "Keep messages in all folders for this account on this computer.">
> +<!ENTITY allFoldersOfflineNote.label "Note: Changing this affects all folders in this account. To set individual folders, select the Advanced… button.">

I think instead of "To set individual folders, *select* the Advanced… button." would be "To set individual folders, *use* the Advanced… button." better. Or "To set individual folders, select them through the Advanced… button."

::: mail/locales/en-US/chrome/messenger/prefs.properties
@@ +84,5 @@
>  removeFromServerTitle=Confirm permanent, automatic deletion of messages
>  removeFromServer=This setting will permanently delete old messages from the remote server AND your local storage. Are you sure you want to proceed?
> +
> +confirmSyncChangesTitle=Confirm synchronization changes
> +confirmSyncChanges=Click OK to save any changes you made to Message Synchronizing. Cancel discards your changes.

I think, we should ask the user directly, like:

"The Message Synchronization settings was changed.

Do you want to save them?"

And instead of [Cancel] use [Discard]
(In reply to Richard Marti (:Paenglab) from comment #17)
> Comment on attachment 8936704 [details] [diff] [review]
> 1402558-review-changes2.patch
> 
> Review of attachment 8936704 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You are changing things in mailnews. This affects SM too and you need to add
> also the locale changes for suite.

Done

> 
> ::: mail/locales/en-US/chrome/messenger/am-offline.dtd
> @@ +5,5 @@
> >  <!ENTITY doNotDownloadPop3Movemail.label "To save disk space, do not download:">
> >  <!ENTITY doNotDownloadNntp.label "To save disk space, do not download for offline use:">
> >  <!ENTITY doNotDownloadImap.label "To save disk space, downloading messages from the server and keeping local copies for offline use can be restricted by age or size.">
> > +<!ENTITY allFoldersOffline.label "Keep messages in all folders for this account on this computer.">
> > +<!ENTITY allFoldersOfflineNote.label "Note: Changing this affects all folders in this account. To set individual folders, select the Advanced… button.">
> 
> I think instead of "To set individual folders, *select* the Advanced…
> button." would be "To set individual folders, *use* the Advanced… button."

Done. I prefer your *use* option.

> better. Or "To set individual folders, select them through the Advanced…
> button."
> 
> ::: mail/locales/en-US/chrome/messenger/prefs.properties
> @@ +84,5 @@
> >  removeFromServerTitle=Confirm permanent, automatic deletion of messages
> >  removeFromServer=This setting will permanently delete old messages from the remote server AND your local storage. Are you sure you want to proceed?
> > +
> > +confirmSyncChangesTitle=Confirm synchronization changes
> > +confirmSyncChanges=Click OK to save any changes you made to Message Synchronizing. Cancel discards your changes.
> 
> I think, we should ask the user directly, like:
> 
> "The Message Synchronization settings was changed.
> 
> Do you want to save them?"

Done.

> 
> And instead of [Cancel] use [Discard]

Using "Discard" instead of "Cancel". Also, instead of OK, I'm using "Save". Is that acceptable?

Will attach updated patch
(In reply to Jorg K (GMT+1) from comment #13)
> Comment on attachment 8936704 [details] [diff] [review]
> 1402558-review-changes2.patch
> 
> Review of attachment 8936704 [details] [diff] [review]:
> -----------------------------------------------------------------
> > 
> If you change the string, you need to change the string ID as well, so this
> becomes allFoldersOffline2.

Done

> 
> > +        // A change to the Offline flag to a folder was made
> 
> Nit. Full stop.
> 
> > +    // A change to the allFoldersOffline checkbox was made
> 
> ditto.

Done.

(In reply to :aceman from comment #14)
> Comment on attachment 8936704 [details] [diff] [review]
> 1402558-review-changes2.patch
> 
> Review of attachment 8936704 [details] [diff] [review]:
> -----------------------------------------------------------------
>  
> Do you want to show the prompt in all cases, or only when the user didn't
> visit the Advanced sub dialog and changes happened (due to touching the
> global checkbox)?

Yes, I now agree that only showing the prompt when the "global" checkbox is touched is better since that does a mass change on all the folders in the current account. But I did add one more feature that also causes the prompt if the checkbox is touched and the OK at the bottom of the Sync & Storage screen is clicked. This also helps prevent a mass change to all account folders that the user may not be expecting.

Attaching updated patch now...
Attached patch 1402558-review-changes3.patch (obsolete) — Splinter Review
Attachment #8936704 - Attachment is obsolete: true
Attachment #8936704 - Flags: ui-review?(richard.marti)
Attachment #8936704 - Flags: review?(acelists)
Attachment #8937218 - Flags: review?(richard.marti)
Attachment #8937218 - Flags: review?(acelists)
Comment on attachment 8937218 [details] [diff] [review]
1402558-review-changes3.patch

Looks good, I give ui-r+ because it's better to let aceman make the code review.

Thank you!
Attachment #8937218 - Flags: review?(richard.marti) → ui-review+
Comment on attachment 8937218 [details] [diff] [review]
1402558-review-changes3.patch

Review of attachment 8937218 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this looks good now. I'll move the comments when checking the patch in.

::: mailnews/base/prefs/content/AccountManager.js
@@ +255,5 @@
>  
> +  // Prompt to avoid unexpected folder sync changes.
> +  if ("onLeave" in top.frames["contentFrame"]) {
> +    if (top.frames["contentFrame"].onLeave() == false) {
> +      // Synch changes discarded and remain on Sync & Storage screen.

These comments should have been on the onLeave in am-offline.js .

The code here is generic to load any onLeave() if found in an account manager page.

@@ +1037,5 @@
>      // is saved and replaced by the new one.
>      tree.focus();
>    }
>  
> +  // Provide opportunity to cancel sync changes before moving to another page.

The same.

::: mailnews/base/prefs/content/am-offline.js
@@ +30,5 @@
>      onCheckKeepMsg();
>  }
>  
> +// Store initial offline flag for each folder and the allFoldersOffline
> +// checkbox. Use to restore the flags and checkbox if edits are canceled.

I forgot to say that functions comments should be in the /** */ format. I'll fix this too.
Attachment #8937218 - Flags: review?(acelists) → review+
Assignee: nobody → gds
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(acelists)
OS: Unspecified → All
Product: Thunderbird → MailNews Core
Hardware: Unspecified → All
Target Milestone: --- → Thunderbird 59.0
Version: 56 Branch → Trunk
Cleaned up the comments.
Attachment #8937218 - Attachment is obsolete: true
Attachment #8938685 - Flags: review+
Keywords: checkin-needed
Blocks: 1426893
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/2bbcacab4613
Guard against unexpected sync status changes for all folders in account. r=aceman, ui-r=Paenglab
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
> -<!ENTITY allFoldersOffline.label "Keep messages for this account on this computer">
> -<!ENTITY allFoldersOffline.accesskey "o">
> +<!ENTITY allFoldersOffline2.label "Keep messages in all folders for this account on this computer.">
> +<!ENTITY allFoldersOffline2.accesskey "o">

It’s crystal clear options like these (as with any other) should not have a trailing period.

Please back out or fix this silently and send out a l10n newsgroup message if possible - 15 locales have already copied the error and "messed up" their localization at this point.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/c40ded187da8
Follow-up: remove unwanted trailing full stop from allFoldersOffline2. r=me DONTBUILD
Done.

If it was "crystal clear" that the full stop shouldn't have been there, why did 15 locales mess up their translation and not correct the error in their translation? :-(
> confirmSyncChanges=The Message Synchronization settings was changed

A setting was, settings were. Please fix this too.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/ed82a1c69080
Follow-up: fix grammar error in message. r=me
You need to log in before you can comment on or make changes to this bug.