730236 - "delete from server" filter option should be removed for IMAP accounts

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Johannes Linke]

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]

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]

Can you try some older builds to when this started to appear?

Comment 3

[:aceman]

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]

10.0.2

i can try older builds tomorrow, if that is still wanted.

Comment 5

[:aceman]

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]

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]

Those items are hidden for News, but are not for IMAP and RSS.
News specific items are enabled fine when needed.

Comment 8

[:aceman]

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]

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]

Thanks, but there doesn't seem to be server type attribute in nsMsgSearchScope.

Comment 11

[David :Bienvenu]

(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]

Attachment: check for .server.type

What about this?

Comment 13

[:aceman]

Maybe this broke when IMAP got offline searching? Was there a point when that changed?

Comment 14

[David :Bienvenu]

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]

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]

(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]

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]

my rss feed server type is "rss".

Comment 19

[:aceman]

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]

(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]

Attachment: patch v2

You started talking about localStoreType (or "mailbox" that is only returned from this attribute) :)

Comment 22

[David :Bienvenu]

(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]

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

yes, thx, this seems to do the right thing.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/comm-central/rev/36757b575b09