Closed
Bug 424024
Opened 17 years ago
Closed 17 years ago
Can't delete folders without a msgWindow and a confirmation
Categories
(Thunderbird :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jminta, Assigned: jminta)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
dmosedale
:
review+
dmosedale
:
superreview+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•17 years ago
|
||
This lets me at least delete folders here.
Assignee | ||
Comment 2•17 years ago
|
||
Same as the previous patch, but diffed in the right direction, and with more context.
Assignee: nobody → jminta
Attachment #310676 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #310810 -
Flags: superreview?(dmose)
Attachment #310810 -
Flags: review?(dmose)
Comment 3•17 years ago
|
||
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.
Attachment #310810 -
Flags: superreview?(dmose)
Attachment #310810 -
Flags: superreview-
Attachment #310810 -
Flags: review?(dmose)
Attachment #310810 -
Flags: review-
Assignee | ||
Comment 4•17 years ago
|
||
This patch prevents ConfirmFolderDelete from being called if there's no msgWindow.
Attachment #310810 -
Attachment is obsolete: true
Attachment #312834 -
Flags: superreview?(dmose)
Attachment #312834 -
Flags: review?(dmose)
Comment 5•17 years ago
|
||
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.
Attachment #312834 -
Flags: superreview?(dmose)
Attachment #312834 -
Flags: superreview+
Attachment #312834 -
Flags: review?(dmose)
Attachment #312834 -
Flags: review+
Comment 6•17 years ago
|
||
Also, bonus points for a simple |make check| unit test.
Assignee | ||
Comment 7•17 years ago
|
||
(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]
Assignee | ||
Comment 8•17 years ago
|
||
Note, however, that if/when we get mochitest set up, then the STEEL tests would exercise this codepath.
Comment 9•17 years ago
|
||
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.
Assignee | ||
Comment 10•17 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•