Forward/Reply filter does not update message status for the forwarded/replied message

RESOLVED FIXED in Thunderbird 3.0b3

Status

MailNews Core
Filters
--
major
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: whimboo, Assigned: Magnus Melin)

Tracking

(Depends on: 1 bug, {dataloss})

Trunk
Thunderbird 3.0b3
dataloss
Bug Flags:
blocking1.9.1 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

5.97 KB, patch
standard8
: review+
standard8
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070111 Thunderbird/3.0a1 ID:2007011103

If you create a filter to forward incoming messages to another address the filter doesn't update the X-Mozilla-Status and X-Mozilla-Status2. So forwarded messages don't have the small purple arrow in front of the subject. Same applies for current 1.8 branch builds.
(Assignee)

Updated

11 years ago
OS: Windows XP → All
Hardware: PC → All
Product: Core → MailNews Core
(Reporter)

Comment 1

9 years ago
Still happens with the latest nightly build on trunk.

Using this feature means you will not know if you have already forwarded or replied a message. Adding the dataloss keyword. Something which could be possible for Thunderbird 3?
Severity: normal → major
Flags: blocking1.9.1?
Keywords: dataloss
Summary: Forward filter does not update status for the forwarded message → Forward/Replay filter does not update message status for the forwarded/replied message
Not blocking 1.9.1.

Renom if you disagree.
Flags: blocking1.9.1? → blocking1.9.1-
Summary: Forward/Replay filter does not update message status for the forwarded/replied message → Forward/Reply filter does not update message status for the forwarded/replied message
(Assignee)

Comment 3

9 years ago
Taking.
Assignee: nobody → mkmelin+mozilla
(Assignee)

Comment 4

9 years ago
Created attachment 366194 [details] [diff] [review]
proposed fix
Attachment #366194 - Flags: superreview?(bugzilla)
Attachment #366194 - Flags: review?(bugzilla)
(Assignee)

Comment 5

9 years ago
(The "a the" comment type i already fixed locally.)
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 3.0b3
Attachment #366194 - Flags: superreview?(bugzilla)
Attachment #366194 - Flags: superreview-
Attachment #366194 - Flags: review?(bugzilla)
Comment on attachment 366194 [details] [diff] [review]
proposed fix

>+/**
>+ * Allow filters to automatically reply to a message. The reply message is
>+ * based on the given template.
>+ */
> NS_IMETHODIMP nsMsgComposeService::ReplyWithTemplate(nsIMsgDBHdr *aMsgHdr, const char *templateUri,
>                                              nsIMsgWindow *aMsgWindow, nsIMsgIncomingServer *aServer)

Hmm, could you add these comments to the idl instead (and for the other function)? Maybe add a few comments about the parameters as well.

>   pMsgComposeParams->SetType(/* forwardType ? nsIMsgCompType::ForwardInline : */nsIMsgCompType::ForwardAsAttachment);
...
>   pMsgComposeParams->SetOriginalMsgURI(msgUri.get());
...
>-  return pMsgCompose->SendMsg(nsIMsgSend::nsMsgDeliverNow, identity, nsnull, nsnull, nsnull) ;
>+  rv = pMsgCompose->SendMsg(nsIMsgSend::nsMsgDeliverNow, identity, nsnull, nsnull, nsnull);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  return folder->AddMessageDispositionState(aMsgHdr, nsIMsgFolder::nsMsgDispositionState_Forwarded);
> }

I don't understand why nsMsgCompose isn't doing this for us. We've set the type to forward, and the original message uri, so nsMsgCompose::ProcessReplyFlags should be setting the forward state on successful send.

In the reply case, we seem to be sending a new message rather than a reply, which seems a bit strange.
(Assignee)

Comment 7

9 years ago
The immediate "cause" is that in nsMsgCompose we have

nsMsgComposeSendListener::OnStopSending

  nsCOMPtr<nsIMsgCompose> msgCompose = do_QueryReferent(mWeakComposeObj, &rv);
  if (msgCompose) {
  ...
    msgCompose->ProcessReplyFlags();

... but the if(msgCompose) is falsy. So ProcessReplyFlags will never get called.

mWeakComposeObj is set though. Don't know what to make if this... so suggestions welcome.

Comment 8

9 years ago
sounds like the compose object has gone away already - perhaps because there's no compose window or js code holding a reference to it?
(Assignee)

Comment 9

9 years ago
Ok, thx. 

So, either I could 
a) do like in the original patch
b) in the original patch, call pMsgCompose->ProcessReplyFlags(); instead of setting disposition
c) figure out how to keep the compose object around

I think I'd like a) the best, and have no idea if c) is even desirable
(Assignee)

Comment 10

9 years ago
(In reply to comment #6)
> In the reply case, we seem to be sending a new message rather than a reply,
> which seems a bit strange.

Well, we are sending a new message - though conceptually it's a reply.
(Assignee)

Comment 11

9 years ago
Created attachment 367855 [details] [diff] [review]
proposed fix, v2

Not *that* much changed from the previous version...
Attachment #366194 - Attachment is obsolete: true
Attachment #367855 - Flags: superreview?(bugzilla)
Attachment #367855 - Flags: review?(bugzilla)
Comment on attachment 367855 [details] [diff] [review]
proposed fix, v2

Sorry for missing the comments on the bug. 

>+  return folder->AddMessageDispositionState(aMsgHdr, nsIMsgFolder::nsMsgDispositionState_Replied);

>+  return folder->AddMessageDispositionState(aMsgHdr, nsIMsgFolder::nsMsgDispositionState_Forwarded);

I think we'll go with this, but we should put a comment in each case explaining why we are doing it "manually" at that time and the compose code isn't.

r/sr=me with comments added.
Attachment #367855 - Flags: superreview?(bugzilla)
Attachment #367855 - Flags: superreview+
Attachment #367855 - Flags: review?(bugzilla)
Attachment #367855 - Flags: review+
(Assignee)

Comment 13

9 years ago
changeset:   2245:c662c338ea41
http://hg.mozilla.org/comm-central/rev/c662c338ea41

... and of course i noticed my typos after committing:(
changeset:   2246:0485638c089a
http://hg.mozilla.org/comm-central/rev/0485638c089a

->FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.