326902 - Forward action in message filters doesn't work with RSS feeds due to no identity for RSS

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Oscar Manuel Gómez Senovilla]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; es-ES; rv:1.7.11) Gecko/20050729
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; es-ES; rv:1.7.11) Gecko/20050729

I try to define a filter for when a RSS feed among the subscribed arrives, I am sent an email. 

Reproducible: Always

Steps to Reproduce:
1. Create a rss feed
2. Add a message filter for the rss account, choosing as action "forward", and the email address to forward the feed to
3. Enable (temporarily) logging in the filter management dialog, to keep track of actions.
4. Check action log when you find new messages

Actual Results:  
I've enabled log, and it shows the filter being executed successfully, but no email arrives, but no email action seems to be performed. The default smtp server (which I can control) has nothing in its logs.


Forwarding action works with email accounts, but it seems it doesn't work with rss feeds.

Comment 1

[David :Bienvenu]

yep, I discovered this yesterday. It might be the case that we can't find an account to use; I'm not sure yet.

Comment 3

[ovidiu]

possible dupe bug 426507

Comment 5

[Wayne Mery (:wsmwk)]

david, do you still want to be assigned?

Comment 6

[David :Bienvenu]

yes, thx for reminding me about this bug. I bet it's that there's no identity for the rss account, and I'm working on a similar bug with archive and rss feeds.

Comment 7

[Wayne Mery (:wsmwk)]

:Bienvenu said in comment #6)
> yes, thx for reminding me about this bug. I bet it's that there's no identity
> for the rss account, and I'm working on a similar bug with archive and rss
> feeds.

kill 2 birds with one stone?

Comment 8

[Ludovic Hirlimann [:Usul]]

David still working on this ?

Comment 9

[Wayne Mery (:wsmwk)]

bienvenu, are those issues resolved?

(In reply to Wayne Mery (:wsmwk) from comment #7)
> :Bienvenu said in comment #6)
> > yes, thx for reminding me about this bug. I bet it's that there's no identity
> > for the rss account, and I'm working on a similar bug with archive and rss
> > feeds.
> 
> kill 2 birds with one stone?

Comment 10

[David :Bienvenu]

My guess is that this is still broken

nsMsgComposeService::ForwardMessage() gets the default identity for the account, but if there are no identities, it won't use the profile default identity. Probably pretty easy to fix.

Comment 11

[:aceman]

David, are you working on it?
From your description, if there is no identity (at http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgComposeService.cpp#1249
), should we GetDefaultAccount() and try to get an identity from that?

Is that what you meant? Or what is the profile default identity?

Comment 12

[David :Bienvenu]

(In reply to :aceman from comment #11)
> David, are you working on it?
> From your description, if there is no identity (at
> http://mxr.mozilla.org/comm-central/source/mailnews/compose/src/
> nsMsgComposeService.cpp#1249
> ), should we GetDefaultAccount() and try to get an identity from that?
> 
> Is that what you meant? Or what is the profile default identity?

nsMsgComposeService::GetDefaultIdentity does that for you, so if there's no identity for the account, you can call this.

Comment 13

[:aceman]

Ok, thanks. Should I take this?

Comment 14

[David :Bienvenu]

(In reply to :aceman from comment #13)
> Ok, thanks. Should I take this?

sure, please, go for it!

Comment 15

[:aceman]

Attachment: patch

Before the patch I created a filter that matched on a RSS message and Forwarded the message to an address. I run that filter manually on a RSS folder. I got a dialog with "Message send failed".

After the patch I get this:
The RSS message gets the orange arrow icon indicating it was forwarded.
A) If TB was offline, the prepared forward message was nowhere to be found (I expected it to be in Outbox).
B) If TB was online, the forward message was sent fine and Sent folder was created under Local Folders and the message got stored there. I received it fine on another account and the sender was the identity from the default account.

Is A a known bug different from this one?

In the file I see some calls to GetDefaultIdentity do not check for success. I added getting the return value (rv = ) where it looked like it was intended just missing. But I left other places untouched as the error checking was not that straightforward there. That is not a problem as it is not part of this bug.

Comment 16

[:aceman]

To be more precise, in case B) the message went into Sent on Local Folders because that was where the default account was set to put sent messages. After fixing this in account settings, the sent message was stored into Sent on the proper default account (from which it took the identity). So that is all OK.

Comment 17

[David :Bienvenu]

Comment on attachment 606909 [details] [diff] [review]
patch

right, yes, that's what I was thinking.

you can simply return defaultAccount->GetDefaultIdentity(_retval);
-      defaultAccount->GetDefaultIdentity(_retval);
+      rv = defaultAccount->GetDefaultIdentity(_retval);

and better to use an early return here:

  nsresult rv;
   nsCOMPtr<nsIMsgAccountManager> accountManager = do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv);
   if (NS_SUCCEEDED(rv) && accountManager)
 
NS_ENSURE_SUCCESS(rv, rv);

then you can remove the if(NS_SUCCEEDED(rv)... line

Comment 18

[:aceman]

Attachment: patch v2

Comment 19

[David :Bienvenu]

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

looks good, thx.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/comm-central/rev/a8a88d3f1bd4