424024 - Can't delete folders without a msgWindow and a confirmation

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Joey Minta]

If one wanted to delete folders in the background, this, it seems, is impossible.  For instance, local folders will simply refuse to do so without a msgWindow. [1].  Moreover, even if a msgWindow is provided, it insists on popping up a confirmation dialog, which for background actions is undesirable. [2]

nsIMsgFolder::deleteSubFolders should just delete the folders if there is no msgWindow provided.

(For further aggravation, the failure in trashFolder->CopyFolder was just swallowed, giving me no indication why the folder wasn't deleted.)

[1] http://mxr.mozilla.org/seamonkey/source/mailnews/local/src/nsLocalMailFolder.cpp#983
[2] http://mxr.mozilla.org/seamonkey/source/mailnews/local/src/nsLocalMailFolder.cpp#1834

Comment 1

[Joey Minta]

Attachment: something like this

This lets me at least delete folders here.

Comment 2

[Joey Minta]

Attachment: patch v1

Same as the previous patch, but diffed in the right direction, and with more context.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 310810 [details] [diff] [review]
patch v1

I agree that it should be possible to do deletions without confirmation, but this is the fix: it changes the semantics of ConfirmFolderDeletion to ConfirmFolderDeletionConditionally.  The right thing to do is make it so that ConfirmFolderDeletion is not called in such cases.

Comment 4

[Joey Minta]

Attachment: patch v2

This patch prevents ConfirmFolderDelete from being called if there's no msgWindow.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 312834 [details] [diff] [review]
patch v2

r+sr=dmose.  ccing bienvenu on the off chance that he has philosophical objections to this.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Also, bonus points for a simple |make check| unit test.

Comment 7

[Joey Minta]

(In reply to comment #6)
> Also, bonus points for a simple |make check| unit test.
> 
This doesn't seem easy to accomplish.  In particular, getting folders to appear in xpcshell is hard.
js> const Cc = Components.classes;
js> const Ci = Components.interfaces;
js> var acctMgr = Cc["@mozilla.org/messenger/account-manager;1"].getService(Ci.nsIMsgAccountManager);
js> acctMgr.createLocalMailAccount();
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /Users/jminta/mozhack/hg-rdf/mozilla/mailnews/base/util/nsMsgIncomingServer.cpp, line 888
WARNING: NS_ENSURE_TRUE(aFile) failed: file /Users/jminta/mozhack/hg-rdf/mozilla/xpcom/io/nsLocalFileOSX.mm, line 1518
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /Users/jminta/mozhack/hg-rdf/mozilla/mailnews/base/src/nsMsgAccountManager.cpp, line 866
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /Users/jminta/mozhack/hg-rdf/mozilla/mailnews/base/util/nsMsgIncomingServer.cpp, line 888
WARNING: NS_ENSURE_TRUE(aFile) failed: file /Users/jminta/mozhack/hg-rdf/mozilla/xpcom/io/nsLocalFileOSX.mm, line 1518
WARNING: NS_ENSURE_TRUE(whichURLRef) failed: file /Users/jminta/mozhack/hg-rdf/mozilla/xpcom/io/nsLocalFileOSX.mm, line 2383
WARNING: NS_ENSURE_TRUE(!pathKey.IsEmpty()) failed: file /Users/jminta/mozhack/hg-rdf/mozilla/mailnews/base/src/nsMsgFolderCache.cpp, line 291
uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgAccountManager.createLocalMailAccount]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: typein :: <TOP_LEVEL> :: line 4"  data: no]

Comment 8

[Joey Minta]

Note, however, that if/when we get mochitest set up, then the STEEL tests would exercise this codepath.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

I'd argue that the right thing to do is to write a test for createLocalMailAccount and fix the code so that the test works.  That'd be entirely reasonable to spin off a separate bug for, though.

Though we definitely want mochitest as well.

Comment 10

[Joey Minta]

Checking in mailnews/local/src/nsLocalMailFolder.cpp;
/cvsroot/mozilla/mailnews/local/src/nsLocalMailFolder.cpp,v  <--  nsLocalMailFolder.cpp
new revision: 1.574; previous revision: 1.573
done