Last Comment Bug 730236 - "delete from server" filter option should be removed for IMAP accounts
: "delete from server" filter option should be removed for IMAP accounts
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: 10
: All All
: -- normal (vote)
: Thunderbird 13.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-24 02:27 PST by Johannes Linke
Modified: 2012-03-05 17:03 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
check for .server.type (1.49 KB, patch)
2012-02-24 14:29 PST, :aceman
mozilla: review-
Details | Diff | Review
patch v2 (1.58 KB, patch)
2012-02-27 11:58 PST, :aceman
mozilla: review+
Details | Diff | Review

Description Johannes Linke 2012-02-24 02:27:54 PST
when i tested it, the filter option "delete from server" did not do anything on IMAP accounts.

in my understanding, an IMAP account in thunderbird should be a "mirror" of the server content. therefore, the description "delete from server" is similar to "delete the message".

i suggest removing the "delete from server"-filter option for IMAP accounts, as it does not work and if it would, it would do the same as "delete the message".
Comment 1 :aceman 2012-02-24 07:03:02 PST
The action is even called "Delete from POP server".
There is a similar one "Fetch from POP server".

I can't judge if the actions are really unreasonable on IMAP therefore I need confirmation from dbienvenu. I can then look at this.

It actually should be enabled only for POP3 as per this file:
http://hg.mozilla.org/comm-central/file/ae193b5797b8/mailnews/base/search/content/searchWidgets.xml#l67

Maybe the determining if this account is POP3 has broken.
Comment 2 :aceman 2012-02-24 07:04:46 PST
Can you try some older builds to when this started to appear?
Comment 3 :aceman 2012-02-24 07:23:21 PST
I smell a regression here, just opening the filter list dialog
throws several errors in the Error console, like this:
Timestamp: 24.2.2012 16:19:25
Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgFolder.getStringProperty]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/folderPane.js :: getSmartFolderName :: line 2468"  data: no]
Source File: chrome://messenger/content/folderPane.js
Line: 2470

Can well be some of my recent patches (the afolder one, or some of the null-args).

However, the reporter filed this under TB10 and those patches are in TB13.

Reporter, what is your exact Thunderbird version?
Comment 4 Johannes Linke 2012-02-24 07:36:35 PST
10.0.2

i can try older builds tomorrow, if that is still wanted.
Comment 5 :aceman 2012-02-24 07:39:13 PST
OK, then I must be seeing something else (I am on TB13).

I can check in TB10 myself, you don't need to do anything now. But the problem still persists in TB13, it just is some other breakage.
Comment 6 :aceman 2012-02-24 07:48:38 PST
Great I now can't even see those errors again.

Can it be those only appeared because I created an IMAP account and didn't restart TB? After restart, no signs of those messages.

But the "Delete from POP server" action bug is there.
Comment 7 :aceman 2012-02-24 11:12:10 PST
Those items are hidden for News, but are not for IMAP and RSS.
News specific items are enabled fine when needed.
Comment 8 :aceman 2012-02-24 11:15:52 PST
So maybe enabling those items when Components.interfaces.nsMsgSearchScope.offlineMailFilter = true is no longer enough? Can it test some other property?
Comment 9 David :Bienvenu 2012-02-24 13:17:45 PST
you'd need to check the server type. The scope for terms is the same because the filter is run locally, but the actions more depend on the server type.
Comment 10 :aceman 2012-02-24 13:29:45 PST
Thanks, but there doesn't seem to be server type attribute in nsMsgSearchScope.
Comment 11 David :Bienvenu 2012-02-24 13:45:02 PST
(In reply to :aceman from comment #10)
> Thanks, but there doesn't seem to be server type attribute in
> nsMsgSearchScope.

right, but the filter widget does know the incoming server type, iirc.
Comment 12 :aceman 2012-02-24 14:29:55 PST
Created attachment 600525 [details] [diff] [review]
check for .server.type

What about this?
Comment 13 :aceman 2012-02-24 14:31:05 PST
Maybe this broke when IMAP got offline searching? Was there a point when that changed?
Comment 14 David :Bienvenu 2012-02-27 10:14:23 PST
Comment on attachment 600525 [details] [diff] [review]
check for .server.type

that does work better, but you'll still have an issue with the global inbox, i.e., the local folders inbox. Filters are supposed to run for all the pop3 accounts that put their mail into the global inbxox, and with this patch, you can't have a local folders filter that deletes pop3 mail from the server. In theory, I suppose you could check if the folder.server.type is also mailbox:

Let me know if you need more info about the global inbox...
Comment 15 :aceman 2012-02-27 11:09:18 PST
Thanks. If I only check for "mailbox" it will also match RSS feeds.

Should I check for "mailbox" but exclude RSS? Would that be proper check?
Comment 16 David :Bienvenu 2012-02-27 11:13:06 PST
(In reply to :aceman from comment #15)
> Thanks. If I only check for "mailbox" it will also match RSS feeds.
> 
> Should I check for "mailbox" but exclude RSS? Would that be proper check?

I thought rss accounts had an "rss" type
Comment 17 :aceman 2012-02-27 11:25:36 PST
Yeah, that was my first try, I checked for "mailbox" but it was true also in RSS. The prefs.js file confirms it too. So I switched to "pop3".
Comment 18 David :Bienvenu 2012-02-27 11:27:13 PST
my rss feed server type is "rss".
Comment 19 :aceman 2012-02-27 11:34:16 PST
Server.type yes, but then that never returns "mailbox" for anything.
Server.localStoreType is "mailbox", for pop3 and RSS. Or am I missing something?

So what I am proposing is:
(gFilterList.folder.server.localStoreType == "mailbox") &&
(gFilterList.folder.server.type != "rss")
Comment 20 David :Bienvenu 2012-02-27 11:37:33 PST
(In reply to :aceman from comment #19)
> Server.type yes, but then that never returns "mailbox" for anything.
> Server.localStoreType is "mailbox", for pop3 and RSS. Or am I missing
> something?
> 
> So what I am proposing is:
> (gFilterList.folder.server.localStoreType == "mailbox") &&
> (gFilterList.folder.server.type != "rss")

Oh, sorry, didn't realize you were talking about localStoreType.

that would work, or I think you could simply check

server.type=="pop3" || server.type=="none" because the local folders account has type "none"
Comment 21 :aceman 2012-02-27 11:58:46 PST
Created attachment 601000 [details] [diff] [review]
patch v2

You started talking about localStoreType (or "mailbox" that is only returned from this attribute) :)
Comment 22 David :Bienvenu 2012-02-27 12:01:09 PST
(In reply to :aceman from comment #21)
> You started talking about localStoreType (or "mailbox" that is only returned
> from this attribute) :)

my mistake, I meant to say server.type "none"
Comment 23 David :Bienvenu 2012-03-05 09:15:02 PST
Comment on attachment 601000 [details] [diff] [review]
patch v2

yes, thx, this seems to do the right thing.
Comment 24 Ryan VanderMeulen [:RyanVM] 2012-03-05 17:03:01 PST
http://hg.mozilla.org/comm-central/rev/36757b575b09

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