Last Comment Bug 809990 - Don't set the offline folder flag for SPAM and TRASH folders
: Don't set the offline folder flag for SPAM and TRASH folders
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 22.0
Assigned To: Magnus Melin
:
Mentors:
Depends on:
Blocks: tb-gmailWIP
  Show dependency treegraph
 
Reported: 2012-11-08 12:12 PST by Kent James (:rkent)
Modified: 2013-03-04 11:55 PST (History)
7 users (show)
mkmelin+mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (11.92 KB, patch)
2013-01-27 02:55 PST, Magnus Melin
irving: review+
Details | Diff | Splinter Review

Description Kent James (:rkent) 2012-11-08 12:12:49 PST
In setting up an initial Gmail account, both the SPAM and TRASH folders have the "use offline" flag set. We should not be downloading messages by default for these folders. The "use offline" should match whatever the gloda defaults are for which folders to index.
Comment 1 Magnus Melin 2013-01-27 02:55:52 PST
Created attachment 706857 [details] [diff] [review]
proposed fix

Successful try run here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=c66481282cbf
Comment 2 :Irving Reid (No longer working on Firefox) 2013-02-26 11:26:14 PST
Comment on attachment 706857 [details] [diff] [review]
proposed fix

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

I'm OK with this being checked in with the comments fixed; if you decide to change the NS_ENSURE_SUCCESS please send it back to me for a quick re-review.

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +968,5 @@
> +      // not always the case.
> +      uint32_t flags = 0;
> +      child->GetFlags(&flags);
> +
> +      // Make the newly created folder is flagged for offline use if the

Wording:

// Set the offline use flag for the newly created folder if the

@@ +969,5 @@
> +      uint32_t flags = 0;
> +      child->GetFlags(&flags);
> +
> +      // Make the newly created folder is flagged for offline use if the
> +      // offline_download prefis true, unless it's the Trash or Junk folder.

prefis -> preference is

@@ +974,5 @@
> +      if (!(flags & (nsMsgFolderFlags::Trash | nsMsgFolderFlags::Junk)))
> +      {
> +        nsCOMPtr<nsIImapIncomingServer> imapServer;
> +        rv = GetImapIncomingServer(getter_AddRefs(imapServer));
> +        NS_ENSURE_SUCCESS(rv, rv);

Do we really want to return early if this fails, or do we just want to give up on setting the flag but continue with the rest of the method?
Comment 3 Magnus Melin 2013-03-03 12:57:36 PST
(In reply to Irving Reid (:irving) from comment #2)
> Do we really want to return early if this fails, or do we just want to give
> up on setting the flag but continue with the rest of the method?

A failure would be pretty strange, especially given that the same thing is used in AddSubfolderWithPath which is called a few lines earlier, and that method is required to succeed to hit the code i'm adding. So I think I prefer the patch as is, with NS_ENSURE_SUCCESS(rv, rv);
Comment 4 Magnus Melin 2013-03-04 11:55:15 PST
http://hg.mozilla.org/comm-central/rev/e115168a56b6 -> FIXED

Note You need to log in before you can comment on or make changes to this bug.