Closed Bug 473373 Opened 11 years ago Closed 11 years ago

Archive function doesn't work if set to archive to local folders.


(MailNews Core :: Backend, defect)

Not set


(Not tracked)

Thunderbird 3.0b2


(Reporter: standard8, Assigned: Bienvenu)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

I set up my archive options to archive to local folders (obviously I didn't test this when I reviewed the bug). When I click archive, an Archives folder is created, but no sub-folders, and the message isn't moved. On the error console I'm getting:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80550013 [nsIMsgFolder.createStorageIfMissing]"  nsresult: "0x80550013 (<unknown>)"  location: "JS frame :: chrome://messenger/content/mailWindowOverlay.js :: anonymous :: line 1066"  data: no]

This should block, as people who want to store archives locally, won't be able to.
Flags: blocking-thunderbird3+
actually, the archives folder is created, and the sub-folders, but we're returning an error when none occurred. I have my suspicions as to what the error is but I need to debug it more.
Attached patch proposed fix (obsolete) — Splinter Review
nsLocalMailFolder::CreateStorageIfMissing shouldn't treat existing folders as an error. The folders exist because of AddSubfolder, which for local folders creates the folder.  In theory, we don't need to call CreateStorageIfMissing for local folders, but I'd need to verify that works. I'm not sure if that would make the js cleaner or messier.

also, the folder creation logic shouldn't expect CreateStorageIfMissing to be async for local folders.

requesting sr just for the nsLocalMailFolder change.
Attachment #356796 - Flags: superreview?(bugzilla)
Attachment #356796 - Flags: review?(bugzilla)
Comment on attachment 356796 [details] [diff] [review]
proposed fix

This works - the only issue I see now is when the Archives folder or year folders are in existence and a new sub folder is created, the folder pane isn't updated. 

>+        if (isImap)
>+          return;
>+        else
>+          dstFolder = subfolder;

nit: no need for else statement.
Attached patch proposed fixSplinter Review
create all the folders the same way...
Attachment #356796 - Attachment is obsolete: true
Attachment #357312 - Flags: review?(bugzilla)
Attachment #356796 - Flags: superreview?(bugzilla)
Attachment #356796 - Flags: review?(bugzilla)
Attachment #357312 - Attachment is patch: true
Attachment #357312 - Flags: superreview+
Attachment #357312 - Flags: review?(bugzilla)
Attachment #357312 - Flags: review+
Comment on attachment 357312 [details] [diff] [review]
proposed fix

That's now working.

>+      archiveFolderUri += "/" + msgYear;
>+      subFolder = GetMsgFolderFromUri(archiveFolderUri, false);
>+      if (!subFolder.parent)
>       {
>-        subfolder = archiveFolder.addSubfolder(msgYear);
>-        subfolder.createStorageIfMissing(this);
>-        return;
>+        subFolder = GetMsgFolderFromUri(archiveFolderUri, false);
>+        subFolder.createStorageIfMissing(this);

You're getting the subFolder twice here, I don't think you need to.
fix checked in, with extra getting of subFolder removed.
Closed: 11 years ago
Resolution: --- → FIXED
Blocks: 473212
Duplicate of this bug: 473597
You need to log in before you can comment on or make changes to this bug.