Last Comment Bug 765820 - Make MDN (return receipts) work for non-standard headers too, and make the MDN confirmation message say which addresses the receipt will be sent to.
: Make MDN (return receipts) work for non-standard headers too, and make the MD...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.13
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-18 10:31 PDT by Philip Chee
Modified: 2012-07-11 11:00 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 Proposed fix. (9.79 KB, patch)
2012-06-18 10:44 PDT, Philip Chee
no flags Details | Diff | Splinter Review
Patch v1.1 (9.80 KB, patch)
2012-06-22 08:51 PDT, Philip Chee
iann_bugzilla: review+
Details | Diff | Splinter Review
Patch v1.2 fix labels. r=IanN. (9.69 KB, patch)
2012-07-08 07:06 PDT, Philip Chee
philip.chee: review+
mnyromyr: superreview+
Details | Diff | Splinter Review

Description Philip Chee 2012-06-18 10:31:16 PDT
Planned Fixes in this bug:
1. Port parts of Bug 568447:
1.1 Make it work correctly with the non-standard Return-Receipt-To.
1.2 The MDN bar does not need to be hidden for deleted IMAP messages.
2. Ports Bug 360800:
2.1 Make the MDN confirmation dialog say which addresses the receipt will be sent to (can be multiple).

From Bug 568447:

> Fix the nits, and also i made it work correctly with the non-standard
> Return-Receipt-To. (Strangely enough i had a mail from an outlook user, in
> one mail its the standard header, in the next the non-standard, go figure...)

> Also i don't think the mdn bar needs to be hidden for deleted imap messages
> if they are showing. It probably made sense when it was a dialog...
Comment 1 Philip Chee 2012-06-18 10:44:33 PDT
Created attachment 634102 [details] [diff] [review]
Patch v1.0 Proposed fix.
Comment 2 neil@parkwaycc.co.uk 2012-06-19 02:40:59 PDT
Comment on attachment 634102 [details] [diff] [review]
Patch v1.0 Proposed fix.

>-<!ENTITY mdnBarIgnoreButton.label "Ignore Request">
>-<!ENTITY mdnBarSendButton.label "Send Receipt">
>+<!ENTITY mdnBarIgnoreButton2.label "Ignore">
>+<!ENTITY mdnBarIgnoreButton2.accesskey "I">
>+<!ENTITY mdnBarSendButton2.label "Send Return Receipt">
>+<!ENTITY mdnBarSendButton2.accesskey "S">
Why are we changing the labels?

>+                 aMimeHdr.extractHeader("Return-Receipt-To", false); // not
not what?

>                  class="msgNotificationBarText">&mdnBarMessage.label;</description>
Didn't you get rid of this entity?
Comment 3 Philip Chee 2012-06-19 02:49:48 PDT
> Why are we changing the labels?
No idea. I'll revert this labels

> not what?
or not RFC 3798.

>>                  class="msgNotificationBarText">&mdnBarMessage.label;</description>
> Didn't you get rid of this entity?
Sigh. insufficient qrefresh. New patch RSN.
Comment 4 Philip Chee 2012-06-22 08:51:24 PDT
Created attachment 635769 [details] [diff] [review]
Patch v1.1

>>-<!ENTITY mdnBarIgnoreButton.label "Ignore Request">
>>-<!ENTITY mdnBarSendButton.label "Send Receipt">
>>+<!ENTITY mdnBarIgnoreButton2.label "Ignore">
>>+<!ENTITY mdnBarIgnoreButton2.accesskey "I">
>>+<!ENTITY mdnBarSendButton2.label "Send Return Receipt">
>>+<!ENTITY mdnBarSendButton2.accesskey "S">
> Why are we changing the labels?
Reverted labels.

>>+                 aMimeHdr.extractHeader("Return-Receipt-To", false); // not
> not what?
Fixed comment.

>>                  class="msgNotificationBarText">&mdnBarMessage.label;</description>
> Didn't you get rid of this entity?
Removed for sure this time.
Comment 5 Ian Neal 2012-07-07 06:52:18 PDT
Comment on attachment 635769 [details] [diff] [review]
Patch v1.1

>+++ b/suite/locales/en-US/chrome/mailnews/messenger.dtd
>+<!ENTITY mdnBarIgnoreButton.label "Ignore">
>+<!ENTITY mdnBarIgnoreButton.accesskey "I">
>+<!ENTITY mdnBarSendButton.label "Send Return Receipt">
>+<!ENTITY mdnBarSendButton.accesskey "S">
I guess the meaning of these labels is not changing so probably no need to change the entity name, but maybe Neil was suggesting you didn't need to change the labels at all, just add the accesskeys.

>+++ b/suite/mailnews/mailWindowOverlay.js

>+    var mdnBarMsg = document.getElementById("mdnBarMessage");
>+    var barMsg;
>+    // If the return receipt doesn't go to the sender address, note that in the
>+    // notification.
>+    if (mdnAddr != fromAddr)
>+      barMsg = gMessengerBundle.getFormattedString("mdnBarMessageAddressDiffers",
>+                                                   [authorName, mdnAddr]);
>+    else
>+      barMsg = gMessengerBundle.getFormattedString("mdnBarMessageNormal",
>+                                                   [authorName]);
>+    mdnBarMsg.textContent = barMsg;
Nit: You could just inline mdnBarMsg.
r=me as long as you double check with Neil.
Comment 6 Philip Chee 2012-07-08 07:06:26 PDT
Created attachment 640066 [details] [diff] [review]
Patch v1.2 fix labels. r=IanN.

> I guess the meaning of these labels is not changing so probably no need to 
> change the entity name, but maybe Neil was suggesting you didn't need to change 
> the labels at all, just add the accesskeys.
Fixed.

> Nit: You could just inline mdnBarMsg.
Fixed.

> r=me as long as you double check with Neil.
Neil confirms that he doesn't want the label text changed, just add the accesskeys.

New patch attached.
Comment 7 Karsten Düsterloh 2012-07-09 13:20:19 PDT
Comment on attachment 640066 [details] [diff] [review]
Patch v1.2 fix labels. r=IanN.

>+/*
>+ * This function handles all mdn response generation (ie, imap and pop).
>+ * For pop the msg uid can be 0 (ie, 1st msg in a local folder) so no
>+ * need to check uid here. No one seems to set mimeHeaders to null so
>+ * no need to check it either.
>+*/

I don't actually see the point here, especially if the last line is misaligned. :-P
Comment 8 Philip Chee 2012-07-11 11:00:57 PDT
> I don't actually see the point here, especially if the last line is misaligned. :-P
Alignment fixed on checkin.

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/06898b5bcf86

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