144296 - MDN: return receipts not on for different ways to bring up compose window

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Backend, defect)

Description

[grylchan]

I'm using 2002-05-12-20-XX-PR1 on windows nt 4.0
but I'm sure this applies to trunk,branch, and all os's.

If you have 'when sending a message always request a return receipt'
on and you compose a mesg any of these ways:
  -click on mail to link
  -click on a mail header url
  -from browser: file|Send page or
                 file|send link
  -select a address in your address book and click the compose button
   on the address book
  -address book: file|new mesg

There will be no request for a return receipt tied to that message.
It's not using the user's identity.

It does work for the following:
  -browser, messenger, or composer: file|New mesg, see bug 134280
  -using compose button in messenger

Steps to reproduce:
1.Go to return receipt prefs and check
 'when sending a message always request a return receipt'
2.Compose a mesg by any of the ways listed above

result: go to compose window and select from menu:
        Options|Return Receipt

        Notice it's not checked. If you send the mesg
        it will not prompt the addressee for a return receipt

expected: to have return receipt on for any way to compose
          a mesg

Comment 1

[scottputterman]

Jeff any ideas on how to fix this?

Comment 2

[grylchan]

adding 'edit as new' as another test case where this doesnt work properly.

Comment 3

[grylchan]

*** Bug 156914 has been marked as a duplicate of this bug. ***

Comment 4

[Cavin Song]

Reassigning.

Comment 5

[Cavin Song]

Attachment: Proposed patch, v1

Set/Init return receipt flag in nsMsgCompose::Initialize().

Comment 6

[(not reading, please use [e-mail] instead)]

Comment on attachment 114144 [details] [diff] [review]
Proposed patch, v1

this is similar to, but not exactly the same code as in
nsMsgComposeService::OpenComposeWindow()

      PRBool requestReturnReceipt = PR_FALSE;
      PRInt32 receiptType = nsIMsgMdnGenerator::eDntType;
      if (identity)
      {
	identity->GetRequestReturnReceipt(&requestReturnReceipt);
	identity->GetReceiptHeaderType(&type);
      }
      pMsgCompFields->SetReturnReceipt(requestReturnReceipt);
      pMsgCompFields->SetReceiptHeaderType(receiptType);

I think want to set call Get/SetReceiptHeaderType() too.

also, these four calls return rv, but your patch (and the code in
OpenComposeWindow()) doesn't check.

I think we should add checks.  

I don't think they should ever fail, but can you double check?

if they shouldn't fail, we should fix your patch (and the code in
OpenComposeWindow()) to do:

rv = GetFoo();	// SetFoo();
NS_ENSURE_SUCCESS(rv, rv);

Comment 7

[(not reading, please use [e-mail] instead)]

I think they way you've done this will make it work properly for the cached
compose window, but please make sure.

for more info on that, see http://www.mozilla.org/mailnews/arch/compose/cached.html

Comment 8

[Cavin Song]

Attachment: Proposed patch, v2

Looks like we don't have to set the flag and type in
nsMsgComposeService::OpenComposeWindow() because nsMsgCompose::Initialize() is
always called (by nsMsgComposeService::InitCompose()) in all cases I tested
unless I missed something. Cached compose window works fine with the patch as
well.

Comment 9

[(not reading, please use [e-mail] instead)]

looks good.  two minor nits.  fix them, and then r/sr=sspitzer

1)

is it possible for m_identity to be null in nsMsgCompose::Initialize()?

(I'm not sure if nsMsgComposeService::OpenWindow() is always called first, which
will set the identity to the default identity if not set)

the reason I ask is one difference between your patch and the code you removed
in nsMsgComposeService::OpenComposeWindow() is that in the existing code, if we
don't have an identity, we still call SetReturnReceipt() and SetReceiptHeaderType()

I looked into it, and I think it would be ok not call them if the default values
for m_returnReceipt and m_receiptHeaderType were correct.

m_returnReceipt is, but m_receiptHeaderType is not.  

receiptHeaderType is uninitialized!

can you initialize it in nsMsgCompFields::nsMsgCompFields() to be
nsIMsgMdnGenerator::eDntType?

2)

I think you can now remove

#include "nsIMsgMdnGenerator.h"

from nsMsgComposeService.cpp, since it is no longer used.

Comment 10

[(not reading, please use [e-mail] instead)]

Comment on attachment 114290 [details] [diff] [review]
Proposed patch, v2

minusing, to clear my request queue.

when you have a final patch, I'll give you that quick r/sr

Comment 11

[Cavin Song]

> can you initialize it in nsMsgCompFields::nsMsgCompFields() to be
> nsIMsgMdnGenerator::eDntType?
>
Excellent catch. Thanks.

I'll remove:

#include "nsIMsgMdnGenerator.h"

from nsMsgComposeService.cpp.

Comment 12

[Cavin Song]

Attachment: Proposed patch, v3.

Incorporated comments.

Comment 13

[(not reading, please use [e-mail] instead)]

Comment on attachment 114309 [details] [diff] [review]
Proposed patch, v3.

r/sr=sspitzer

nice work, cavin.

Comment 14

[Cavin Song]

Fix checked in.

Comment 15

[(not reading, please use [e-mail] instead)]

this caused a regression, see bug #195364

(but I think it's just that a default pref is missing)

Comment 16

[(not reading, please use [e-mail] instead)]

actually, it was something else (but equally as small)

see bug #195364 for details.

Comment 17

[(not reading, please use [e-mail] instead)]

the problem was with the implementation (not cavin's)
of nsMsgIdentity::GetReceiptHeaderTypeGetReceiptHeaderType()

cavin must have per-account overrides turned on, which is why he didn't see it.

Comment 18

[(not reading, please use [e-mail] instead)]

but this did cause a regression.

see bug #195367 (reply not quoting)

here's the fix

-    rv = m_identity->GetReceiptHeaderType(&type);
+    rv = m_identity->GetReceiptHeaderType(&receiptType);

Comment 19

[Ninoschka Baca]

Trunk build 2003-03-05: Mac 10.1.5
Trunk build 2003-03-05: WinXP
Verified Fixed. The following scenarios result in a return receipt appearing.

-click on mail to link
-click on a mail header url
-from browser: - file|Send page or
               - file|send link
-from address book: - click the compose button with a card selected
                    - file|new mesg with a card selected