Last Comment Bug 326902 - Forward action in message filters doesn't work with RSS feeds due to no identity for RSS
: Forward action in message filters doesn't work with RSS feeds due to no ident...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Feed Reader (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: Thunderbird 14.0
Assigned To: :aceman
:
Mentors:
: 378774 426507 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-02-12 08:01 PST by Oscar Manuel Gómez Senovilla
Modified: 2012-11-10 11:40 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (4.59 KB, patch)
2012-03-17 15:31 PDT, :aceman
mozilla: feedback+
Details | Diff | Review
patch v2 (5.24 KB, patch)
2012-03-19 13:49 PDT, :aceman
mozilla: review+
Details | Diff | Review

Description Oscar Manuel Gómez Senovilla 2006-02-12 08:01:30 PST
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 2006-02-12 13:10:39 PST
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 2 Magnus Melin 2007-04-25 12:55:16 PDT
*** Bug 378774 has been marked as a duplicate of this bug. ***
Comment 3 ovidiu 2008-06-02 10:38:00 PDT
possible dupe bug 426507
Comment 4 Wayne Mery (:wsmwk, NI for questions) 2009-01-23 08:42:22 PST
*** Bug 426507 has been marked as a duplicate of this bug. ***
Comment 5 Wayne Mery (:wsmwk, NI for questions) 2009-01-23 08:42:39 PST
david, do you still want to be assigned?
Comment 6 David :Bienvenu 2009-01-24 10:14:54 PST
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, NI for questions) 2010-01-10 22:44:25 PST
: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] 2010-04-27 00:50:38 PDT
David still working on this ?
Comment 9 Wayne Mery (:wsmwk, NI for questions) 2012-03-17 06:26:51 PDT
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 2012-03-17 10:18:39 PDT
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 2012-03-17 14:27:44 PDT
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 2012-03-17 14:40:53 PDT
(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 2012-03-17 14:48:09 PDT
Ok, thanks. Should I take this?
Comment 14 David :Bienvenu 2012-03-17 15:12:12 PDT
(In reply to :aceman from comment #13)
> Ok, thanks. Should I take this?

sure, please, go for it!
Comment 15 :aceman 2012-03-17 15:31:55 PDT
Created attachment 606909 [details] [diff] [review]
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 2012-03-18 06:54:06 PDT
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 2012-03-19 08:50:18 PDT
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 2012-03-19 13:49:11 PDT
Created attachment 607295 [details] [diff] [review]
patch v2
Comment 19 David :Bienvenu 2012-03-21 11:47:59 PDT
Comment on attachment 607295 [details] [diff] [review]
patch v2

looks good, thx.
Comment 20 Ryan VanderMeulen [:RyanVM] 2012-03-21 16:41:54 PDT
http://hg.mozilla.org/comm-central/rev/a8a88d3f1bd4

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