Closed Bug 326902 Opened 14 years ago Closed 8 years ago

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

Categories

(MailNews Core :: Feed Reader, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 14.0

People

(Reporter: omgs, Assigned: aceman)

References

Details

Attachments

(1 file, 1 obsolete file)

5.24 KB, patch
Bienvenu
: review+
Details | Diff | Splinter Review
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.
yep, I discovered this yesterday. It might be the case that we can't find an account to use; I'm not sure yet.
Assignee: mscott → bienvenu
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Duplicate of this bug: 378774
possible dupe bug 426507
Duplicate of this bug: 426507
david, do you still want to be assigned?
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.
Status: NEW → ASSIGNED
Component: RSS → Feed Reader
Product: Thunderbird → MailNews Core
: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?
Summary: Forward action in message filters doesn't work with RSS feeds → Forward action in message filters doesn't work with RSS feeds due to no identity for RSS
David still working on this ?
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?
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.
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?
(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.
Ok, thanks. Should I take this?
(In reply to :aceman from comment #13)
> Ok, thanks. Should I take this?

sure, please, go for it!
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #606909 - Flags: feedback?(dbienvenu)
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 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
Attachment #606909 - Flags: feedback?(dbienvenu) → feedback+
Attached patch patch v2Splinter Review
Assignee: dbienvenu → acelists
Attachment #606909 - Attachment is obsolete: true
Attachment #607295 - Flags: review?(dbienvenu)
Comment on attachment 607295 [details] [diff] [review]
patch v2

looks good, thx.
Attachment #607295 - Flags: review?(dbienvenu) → review+
Keywords: checkin-needed
Version: unspecified → Trunk
http://hg.mozilla.org/comm-central/rev/a8a88d3f1bd4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.