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

RESOLVED FIXED in Thunderbird 14.0

Status

MailNews Core
Feed Reader
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: Oscar Manuel Gómez Senovilla, Assigned: aceman)

Tracking

Trunk
Thunderbird 14.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

5.24 KB, patch
Bienvenu
: review+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
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

11 years ago
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

Updated

10 years ago
OS: Linux → All
Hardware: PC → All

Updated

10 years ago
Duplicate of this bug: 378774

Comment 3

9 years ago
possible dupe bug 426507
Duplicate of this bug: 426507
david, do you still want to be assigned?

Comment 6

9 years ago
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

Updated

8 years ago
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?

Comment 10

5 years ago
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.
(Assignee)

Comment 11

5 years ago
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

5 years ago
(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.
(Assignee)

Comment 13

5 years ago
Ok, thanks. Should I take this?

Comment 14

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

sure, please, go for it!
(Assignee)

Comment 15

5 years ago
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.
Attachment #606909 - Flags: feedback?(dbienvenu)
(Assignee)

Comment 16

5 years ago
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

5 years ago
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+
(Assignee)

Comment 18

5 years ago
Created attachment 607295 [details] [diff] [review]
patch v2
Assignee: dbienvenu → acelists
Attachment #606909 - Attachment is obsolete: true
Attachment #607295 - Flags: review?(dbienvenu)

Comment 19

5 years ago
Comment on attachment 607295 [details] [diff] [review]
patch v2

looks good, thx.
Attachment #607295 - Flags: review?(dbienvenu) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Version: unspecified → Trunk
http://hg.mozilla.org/comm-central/rev/a8a88d3f1bd4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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.