Closed Bug 16241 Opened 25 years ago Closed 22 years ago

Implement MDN Return Receipts

Categories

(MailNews Core :: Backend, defect, P2)

Product:
MailNews Core
Component:
Backend
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: fenella, Assigned: jt95070)

References

(Blocks 1 open bug,
URL
)

Details

(Keywords: helpwanted, Whiteboard: [adt2][P1])

Attachments

(4 files, 27 obsolete files)

MDN Task list
1.95 KB, text/html
Details
Complement patch for Mac (mdn disabled)
152.01 KB, patch
sspitzer
: review+
Bienvenu
: superreview+
asa
: approval+
Details | Diff | Splinter Review
patch, more cleanup, testing it now, ready for review.
97.95 KB, patch
sspitzer
: review+
Bienvenu
: superreview+
asa
: approval+
Details | Diff | Splinter Review
installer fixes
2.82 KB, patch
sspitzer
: review+
sspitzer
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
Linux (1999-10-11-14-M11) Commercial
Win32 (1999-10-11-15-M11) Commercial
Mac (1999-10-11-14-M11) Mozilla

This bug tracks the Return Receipt item list features.

Steps:
1. Launch Messenger, select Edit|Preferences|Return Receipts
2. Click on the radio buttons to receive "Both types of receipt"
3. Close the Preference dialog window
4. Relaunch the Preference, it remembers the selected items. But...

Actual result: None of the checked items is enabled.

Expected result: when they are enabled, will be able to receive MDN and DSN
Assignee: phil → jefft
Target Milestone: M15
Reassign to jefft. Not a B1 feature, so setting M15
Status: NEW → ASSIGNED
QA Contact: lchiang → fenella
Summary: Preferences Return Receipts item lists are NOT enabled → [FEATURE] Preferences Return Receipts item lists are NOT enabled
Whiteboard: ETA 04/19/00
move to M16.  Not M15 stoppers
Target Milestone: M15 → M16
Backend codes never been ported over. This invloves filters and all mail 
protocls. Imap handling is quiet different that pop3. Also depends on the server 
capability of supporting user defined flags.
Whiteboard: ETA 04/19/00 → 5 days
adding suresh, because if this doesn't make it, suresh will be ripping out the 
ui.

if it does make it, suresh will be finishing the ui.
I'm considering this the "hook up" backend kind of bug for return receipts
prefs.

I logged two pref UI revision bugs about the return receipts panel according to
revised 05/03/00 mail prefs spec:  bug #38308 and bug #38309
More related bugs... it just gets more confusing:  bug #39770 bug #39791
This requires some work. Marking it M20. Don't think it can make it for final 
release. We need to make sure to take out the choice in the compose window and 
reinstate it when we have the feature implemented.
Target Milestone: M16 → M20
Moving feature bugs to target milestone "Future" for next release consideration.

I'll see if we have a bug to remove return receipts from the compose UI menu.  
If not, I'll create a bug report.
Target Milestone: M20 → Future
Jeff, Please comments by Laurel in bug 39770. Should this bug be marked
Resolved/Latered?
Please advise.
Later....
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → LATER
*** Bug 60493 has been marked as a duplicate of this bug. ***
*** Bug 60493 has been marked as a duplicate of this bug. ***
VERIFIED LATER..
Status: RESOLVED → VERIFIED
*** Bug 63325 has been marked as a duplicate of this bug. ***
We don't use "LATER" anymore when marking bugs, correct?  Should I re-open this
and have it re-triaged?
Normally feature request bugs are left open with severity marked as enhancement.
If the assigned engineer would like help implementing the feature because their
busy doing other things add the 'helpwanted' keyword, if the assigned engineer
isn't going to be working on this feature reassign to nobody@mozilla.org

Also add any other appropriate keywords such as 4xp as this feature used to be
in 4.x and isn't in Mozilla yet.
I'm re-opening this (we don't use the LATER field anymore).
Status: VERIFIED → REOPENED
Resolution: LATER → ---
*** Bug 64774 has been marked as a duplicate of this bug. ***
Assign it to Sheelar
QA Contact: fenella → sheelar
reassign to esther
QA Contact: sheelar → esther
In Januari david.hallowell@ukuug.org made some good points on using the keywords
on this bug. Is there a special reason this is not done? (And what does the 5
days in the status whiteboard mean)

Besides an LDAP enabled addressbook, return receipts seem paramount for business
use of mail applications.

I've been looking into the code for *requesting* DSN and *sending* MDN seems to
be already there. (maybe it needs a lot of rewrite, but I cannot imagine that)
There is already some code for the preferences userinterface. 

If sombody could shed a light on what needs to be done I might be able to help a
little...
It would be nice just to be able to request DSNs on outgoing messages. I don't
even use MDNs.


ofcourse 'imagine' should be 'judge'
*** Bug 75149 has been marked as a duplicate of this bug. ***
*** Bug 81680 has been marked as a duplicate of this bug. ***
(Tidying up a bit.)

Can anyone on the Mail/News team add a quick note regarding what's still to do 
here?

Gerv
Keywords: 4xp, helpwanted
Summary: [FEATURE] Preferences Return Receipts item lists are NOT enabled → Preferences Return Receipts item lists are NOT enabled
Whiteboard: 5 days
Return receipts is a very useful feature for lots of people. IMHO it should be
implemented at least by 1.0
Keywords: mozilla1.0
Absolutely, I need this function everyday! Can live w/o... implemente ASAP.

*** Bug 89070 has been marked as a duplicate of this bug. ***
Adding nsenterprise, this is a feature that will likely be used by many large
organisations currently using communicator 4.x if their mail policy requires
confirmation.
Keywords: nsenterprise
I filed RFE bug 93085 requesting support of DSN receipts only (no MDN). If I
understand teh system properly, this bug (for DSN and MDN) should be marked as
dependent on 93085.
Depends on: 93085
Removing nsenterprise nomination.
Keywords: nsenterprise
*** Bug 103631 has been marked as a duplicate of this bug. ***
This bug has turned into not being about the pref but about the feature so I'm
changing the subject.  Likewise, 93085 has already been created for DSN, so I'm
changing this into an MDN only bug.  I'm also removing the dependency since I'm
taking the DSN out of this bug.

Finally, reassigning to bienvenu and moving to 0.9.7
Assignee: jefft → bienvenu
Status: REOPENED → NEW
No longer depends on: 93085
Priority: P3 → P2
Summary: Preferences Return Receipts item lists are NOT enabled → Implement MDN Return Receipts
Target Milestone: Future → mozilla0.9.7
Blocks: 102231
*** Bug 104339 has been marked as a duplicate of this bug. ***
Keywords: nsbeta1+
Target Milestone: mozilla0.9.7 → mozilla0.9.9
*** Bug 110038 has been marked as a duplicate of this bug. ***
*** Bug 111304 has been marked as a duplicate of this bug. ***
If this bug is still help wanted, I repeat after Gerv: 
Can anyone on the Mail/News team add a quick note regarding what's still to do 
here?

Is far as I understand, we're alreay set corresponding flag in msg db when
Disposition-Notification-To header is present. Remaining tasks to be done is
check if we need to get user confirmation on MDN send and generate/send MDN itself.
I have a couple of questions here:
What the best place for those tasks? Is is MsgDBView of smth else?
How to access all message headers when displaying message? (it's needed to get
few header which is not in MsgDBHdr)
Basically nothing is done for MDN - there might be a couple places where flags
are set appropriately, but that's less than 1% of the work required (there seems
to be this perception that MDN is basically done except for a couple small tasks
- if that were true, we'd finish it - I don't know where this perception comes
from but it's wrong). I can attach a brief task list. There are several places
that code needs to be added, but I don't think the view code is involved. As far
as getting extra headers from a displayed message is concerned, in the past, we
would get extra headers from libmime when displaying a message, but that may no
longer be possible. We might have to attach a mime headers listener to message
display, like that which is used for message reply, which would slow down
message display. Or, we could remember these headers when we parse the message,
which would enlarge the db and slow down imap header download. Or we could hook
into the header listening that goes on in msgHdrViewOverlay.js (but we don't
want to slow that code down). So I'm not sure - that needs to be figured out
carefully.
Attached file MDN Task list — 

    
    

    
  
> We might have to attach a mime headers listener to message
> display, like that which is used for message reply, which would slow down
> message display. Or, we could remember these headers when we parse the message,
> which would enlarge the db and slow down imap header download. Or we could hook
> into the header listening that goes on in msgHdrViewOverlay.js (but we don't
> want to slow that code down). So I'm not sure - that needs to be figured out
> carefully.

What about (in IMAP case) nsImapMailFolder::NormalEndHeaderParseStream()?
When in this method, we can check MsgDBHdr for MDN flag and if it's present,
just pass MDN related headers <somewhere>, where they can be used for MDN stuff 
right_after_message_has_been_displayed. May be this solution is not ideal,
but it has minimal impact on performance :-)
What do you think?



    
    

    
  
nsImapMailFolder::NormalEndHeaderParseStream() can be called months before the
message is displayed :-) - "somewhere" in this approach would have to be the db,
so it would be the same as one of my above suggestions.

    
    

    
  
Yep, it was a bit stupid suggestion... But does storing these headers in dedicated 
storage (say, hashtable or array keyed by message key) is worse than keeping them
in db?
Another idea (sorry, if it's stupid as well) - enable more than one
nsIMsgHeaderSink?


    
    

    
  
I didn't mean to say it was a stupid suggestion (hey, I suggested it too!). The
storage would have to be persistent, on disk, since the app can shut down
between the time nsImapMailFolder::NormalEndHeaderParseStream() gets called and
the user reads the message. So it might as well be the db.

Enabling more than one nsIMsgHdrSink is like adding another mime headers
listener to message display - it means we're listening for headers. Now that I
look at it, I don't see anyone implementing nsIMsgHeaderSink, so I'm not sure
that's still used.



    
    

    
  
> Enabling more than one nsIMsgHdrSink is like adding another mime headers
> listener to message display - it means we're listening for headers.

Is it SO bad? Also, can we attach yet another listener *only* if MDN flag
is set? 

> I don't see anyone implementing nsIMsgHeaderSink, so I'm not sure
> that's still used.

See base/resources/content/msgHdrViewOverlay.js line 217 ;-)

It looks like we're doomed - there's no good solution currently,right?
I wonder how it was implemented in NS4 and can it help us?



    
    

    
  
possibly we could add a listener only if the mdn flag is set - I'd need to look
more closely at it. We're not doomed; we just need to be careful up front and
think through the issues. 4.x worked, as I said before, by allowing us to ask
libmime for the headers at display time. libmime no longer keeps track of the
headers - it went to a notification model, where you got notified of headers as
they were parsed.

    
    

    
  
Another reason to use header listener - when sending back MDN reciept,
we need send headers as part of it...

> We might have to attach a mime headers listener to message
> display, like that which is used for message reply

Could you please point me to the relevant code? I cannot find it myself :-(


    
    

    
  
the compose code uses nsIMimeStreamConverterListener to get notified of headers.
Look at nsMsgQuoteListener::OnHeadersReady. Currently, I don't think there is a
header listener on message display, but I think we could add one. I don't
believe we can have multiple nsIMsgHdrSinks, however, without changing the
header sink code.

    
    

    
  
I think that nsStreamConverter currently cannot have more than one 
nsIMimeStreamConverterListener too :-)
Besides, if we're gonna return all headers in MDN, nsIMsgHeaderSink
seems to be more suitable, isn't it?
Another question - where to put MDN handling stuff (attaching listener,
getting headers, making decision etc) - does nsMessenger::OpenURL
looks good? 



    
    

    
  
I missed one thing: we cannot use MsgHeaderSink, because it looks it has been
designed (or optimized) for exclusive use of header pane (JS code), so it supplies
all headers only when in 'View All Headers' mode...

So we have to use nsIMimeStreamConverterListener (may be we even don't need to
support multiple such listeners). But I can't find yet how to inject our
listener into nsStreamConverter at display time....



    
    

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

    
    

    
  
I can't find any way to use msg reply approach to get message headers, it looks
impossible to me - nsStreamSonverter wants that listener only from nsMsgQuote...


    
    

    
  
Hmm, is it possible to get using message headers from msg cache using one
of those magic params of msg URI (like '?headersOnly') ?
Is it slow?

    
  
Keywords: nsenterprise
Whiteboard: [P1]

    
  
Blocks: 121408

    
    

    
  
FYI, This feature should start getting implemented next week.  I'll reassign to
the new owner as soon as I get the correct bugzilla email address.

    
    

    
  
Is it possible to have a setting in Preferences for "Request a receipt for all 
sent messages"  and   "Request a receipt manually"  so that those who 
usually do not want receipts do not have to always get them, and those 
who do want to get receipts most of the time do not have to manually 
select "Request receipt" to ask for a receipt.     Is Mozilla to support 
requesting both server receipts (your message has been delivered to the 
recipient's mailbox) and read receipts (your message has been 
received)?      And, what are the plans for returning receipts if the sender of 
a message requested one?

    
    

    
  
reassigning to jt95070@netscape.net


Assignee: bienvenu → jt95070
Target Milestone: mozilla0.9.9 → mozilla1.0

    
    

    
  
Accepting...
Status: NEW → ASSIGNED

    
    

    
  
From a discussion with Seth:
if we end up putting any of these settings on the account level, you don't need
to add new methods to nsIMsgIncomingServer to get and set the attributes, you
can use generic methods

instead of having server.foo = true and server->GetFoo(&bool); and extending the
nsIMsgIncomingServer interface and implementing getters and setters, you can
jsut do:
server->GetBoolValue("foo", &bool);
and
server->SetBoolValue("foo", PR_TRUE)l
and from js:
server.setBoolValue("foo", true);
rv = server->GetBoolValue("foo", &bool);
rv = server->SetBoolValue("foo", PR_TRUE);

Seth also says that if we put this at the account level, we should make it an
extension so that it's easy to hide for news (MDN does not apply to news servers)

    
    

    
  
see how the smime extension is implemented (mozilla/extensions/smime), the
S/MIME account specific UI only shows up non-news accounts.

    
    

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

    
    

    
  
I have the pop3 mdn return receipt sort of working noew and I would like to 
start landing my changes little by little (not really for the first one). 
Attached are couple diffs files and a mdn zip file which contains the new mdn 
extension. I would like to have them reviewed as soon as possible so that I can 
check them in to meet the March V deadline. Please bear in mind that a lot of 
code are still in progress. Don't shoot me if things are not quiet working at 
the moment.

    
    

    
  


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Backend MDN prefs and Pop3 work. (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        Zip file for MDN generator component (obsolete)
        — 
      

    


  

    
    

    
  
first comment is that you should define some enums in the idl for the
m_receiptHeaderType and use them instead of naked PRInt32's. It will make the
code much more readable and maintainable.

this code:
NS_IMETHODIMP nsMsgWindow::GetMimeHeaders(nsIMimeHeaders * *mimeHeaders)
+{
+
if(!mimeHeaders)
+
	return NS_ERROR_NULL_POINTER;
+
+
*mimeHeaders = mMimeHeaders;
+
NS_IF_ADDREF(*mimeHeaders);
+
return NS_OK;
+}
+

should be
NS_IMETHODIMP nsMsgWindow::GetMimeHeaders(nsIMimeHeaders * *aMimeHeaders)
{
NS_ENSURE_ARG_POINTER(aMimeHeaders);
NS_IF_ADDREF(*aMimeHeaders = mMimeHeaders);
return NS_OK;
}

You need to define a contract id for the mdn generater, and use that instead of
the cid.

Is this following diff a change, or just reformatting?

-
			nsresult err = MoveIncorporatedMessage(msgHdr, m_mailDB, (const char *)
actionTargetFolderUri, filter, msgWindow);
+
			nsresult err = MoveIncorporatedMessage(msgHdr, m_mailDB,
+                                                       (const char *)
+                                                       actionTargetFolderUri,
+                                                       filter, msgWindow);

looks like you're just reformatting here.

I don't ever see you clear the mime headers field in the nsIMsgWindow. Shouldn't
you clear it after you're done with it so that we don't hold onto them past the
point that we need them? This also reduces the chances of memory leaks
introduced by circular references (I'm not sure what the nsMimeHeaders
implementation holds onto)


    
    

    
  

      
      
      
      

        Attached patch
          
          
        diffs for the mdn directory (obsolete)
        — 
          — Splinter Review
      

    


  
I've attached diffs for the mdn directory so we can read them without zip.

    
  

        Attachment #71845 -
        Attachment is obsolete: true

    
    

    
  
there are lots of tabs in the mdn source files - please fix them. The easiest
way to find them is change your msdev settings (tools | options | tabs) to set
tab size to 8.

    
    

    
  
shuehan is going to start on the pref UI and the account manager extension.

none of it will be on by default, but once the files are added, she will 
provide a small patch that will turn the UI on, so that jefft can use it.

(and jefft can land this small UI patch, once he is ready to land the back end 
code.)

I think the only overlap with jefft's existing patch is the default prefs 
file:  mdn.js

shuehan will be adding that to 
mozilla/mailnews/extensions/mdn/resources/content/mdn.js, but this shouldn't 
cause problems.

adding shuehan to the cc list, to correct me if I got anything wrong.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        New diffs with David's comments (obsolete)
        — 
          — Splinter Review
      

    


  
New diffs with David's comments to define receiptHeaderType and incorporate
incoming return receipt type, adding getter and setter of nsIMimeHeaders to
nsIMsgMailNewsUrl instead of nsIMsgWindow. David, could you also mark 71842 and
71843 attachments obsolete. Apparently, I don't have the permission.

    
    

    
  


  
Sorry for much of the traffics ... I just forgot to put in another David's
comment regarding using CONTRACTID instead of CID.

        Attachment #71842 -
        Attachment is obsolete: true

        Attachment #71843 -
        Attachment is obsolete: true

        Attachment #72015 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        More up-to-date diffs (obsolete)
        — 
          — Splinter Review
      

    


  
Pop3 backend MDN code is pretty much done. MDN report generation and
incorporation features are all implemented. Imap4 backend MDN report generation
is also implemented and working. However, the incorporation part is not done
yet. The patch builds on linux machine too but having problem loading mdn
string bundle.

        Attachment #71884 -
        Attachment is obsolete: true

        Attachment #72018 -
        Attachment is obsolete: true

    
    

    
  
Will this make 0.9.9?  (Mozilla 1.0 is listed as target milestone.)   Is this 
already implemented in the nightly builds?


    
    

    
  
No, it's going in 1.0, not .9.9

    
    

    
  
looks like tabs here:
+    NS_ENSURE_ARG_POINTER(mimeHeaders);
+
NS_IF_ADDREF(*mimeHeaders = mMimeHeaders);
+
return NS_OK;

I think the copyrights should be 2002, not 1998.

+    if (name) {
+        rv = m_mdnBundle->GetStringFromName(name, outString);
+        if (NS_SUCCEEDED(rv)) {
+            rv = NS_OK;
+        }

no need to set rv to NS_OK; any NS_SUCCEEDED(rv) should be OK, so you don't need
to change it.

In +void nsMsgMdnGenerator::StoreImapMDNSentFlag(nsIMsgFolder *folder,
+                                             nsMsgKey key)


the failure returns should be on a separate line for ease in setting
breakpoints, e.g., 

+    if (NS_FAILED(rv)) return;

should be

+    if (NS_FAILED(rv)) 
+       return;

There are a lot of these; could you fix them all? It'll speed up the sr, I promise.
no need to use PR_FREEIF, PR_Free checks for null as well.
+    PR_FREEIF (convbuf);

What's the story with the word_wrap stuff? Are we doing it or not?

    
    

    
  
We should do the word wrap for better MTA compliant. Some legacy MTA may chopped
off the line to 76 or less characters. I am suprised that we didn't do word wrap
before we deliver the final RFC822 messages to the smtp server. If we have more
time we should do it.

    
    

    
  


  
New patch contains fixes with David's comment, duplicate mdn msg filter
creation problem, and imap mdn report incorpation implementation. The feature
is pretty much there once I integrate with Shuehan's UI work. I am still having
problems with loading msgmdn.properties string bundle file on Linux. Could any
Linux gurus help me on this? I just couldn't figure out what went wrong. Thanks
in advance.

        Attachment #72364 -
        Attachment is obsolete: true

    
    

    
  


  
New patch contains fixes with David's comment, duplicate mdn msg filter
creation problem, and imap mdn report incorporation implementation. The feature
is pretty much there once I integrate with Shuehan's UI work. I am still having
problems with loading msgmdn.properties string bundle file on Linux. Could any
Linux gurus help me on this? I just couldn't figure out what went wrong. Thanks
in advance.

    
    

    
  
I am in the process to apply the patch to Mac and update the build system for
it. I have couple comment:

1) Why to you have those Makefile? Makefile.in and makefile.win are all you need!
2) Don't need the MANIFEST file on Mac anymore, instead you need to update the
jar file (I never remember where is it).
3) Why does the file mdn-service.js in not into the resources/content folder?
4)In the file extension/mdn/resources/Makefile.in, you need to add also the
directory locale.
5)You are missing couple Makefile.in in the resources folder and sub folder.
6) Does MDN really need to be in its own DLL, why not merging it into mailnews/base?

    
    

    
  
Makefile shouldn't be there. I guess I farted on creating the patch. I forgot
how MANIFEST and Mac build work in general. I was copying from other component
implementation. mdn-service.js probably shouldn't be in its current place.
Again, I was copying from other component implementation. The Makefile.in seems
not needed to be in the resouces directory as I look into other implementations
such as imap. I guess the way resouces get packaged in linux is sort of
different than windows environment. MDN doesn't need to be in its own dll but
that's sort of decided that we follow the way as smime is implemented. As an
extension that can be deployed as additional enterprise components.

    
    

    
  
Don't know if it is too late or not to get this suggestion in...   In addition 
to the choice to request a receipt in each individual message (like 4.x) is it 
possible to have a preference to "request a receipt for each outgoing message"?

    
  
Blocks: 129416

    
  
Blocks: 129418

    
  
Blocks: 129422

    
    

    
  


  
New patch with linux string bundle fix. 
Answer to Joel: yes, there is a pref for that.

        Attachment #72754 -
        Attachment is obsolete: true

        Attachment #72755 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        MDN feature disabled patch (obsolete)
        — 
          — Splinter Review
      

    


  
This is an MDN disabled patch for the check in purpose. Please review. I hope I
can land the change asap. To enable it, you need to define an environment
variable, export MDN_ENABLED=1, and then tweak rule.mak/rule.mk to add the
-DMDN_ENABED compilation flag.

    
    

    
  
adding myself in cc

    
    

    
  
I don't understand what the file mdn-service.js. When do you need it?

Also, the only MANIFEST file you need on Mac is the one in mdn/build in order go
give other module (mailnews/local) access to nsMsgMdnCID.h BTW, would be nicer
if the content of nsMsgMdnCID.h was part of the ILD file. Why should I have to
include both the interface include (nsIMsgMdnGenerator.h) and nsMsgMdnCID.h when
I want to use a module? 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        to enable pref panel UI (obsolete)
        — 
          — Splinter Review
      

    


  
pref panel and account manager UI has been checked in as not part of build -
this is to turn on the pref panel - see 128962

    
    

    
  
Jeff, you cannot access nsMsgCompUtils.cpp functions from mdn as on Mac we don't
generate a static library for each module. And it not a good modular design.
Etheir you duplicate the function you need into mdn (looks like their are only 2
and they are small) of we need to create an interface in msgCompose to let you
access those function...

    
    

    
  
JF, I am working on a patch which will have nsIMsgMdnGenerator.idl placed in
mailnews/base/public directory. The contract id and cid (not needed really) will
be defined in nsIMsgMdnGenerator.idl too. And I do not need mdn-service.js at
the moment. It can be punted. However, Shuehan is providing one I think and I am
not sure which directory will be landed at the moment. So, we should be cool on
the nsMsgMdnCID.h issue as well as mdn-service.js. If we could have the two
routines in nsMsgCompUtils.cpp exported via IDL that would be great. I believe
smime use one of them too. I'll try to work on exporting it out.

    
    

    
  
Another easier solution would be to move them into mailnews/base/utils. It's the
only static lib in mailnews that we share in almost all mailnews modules.

    
    

    
  
ducarroz asked: 

> I don't understand what the file mdn-service.js. When do you need it?

If that's what I think it is, we do need it.  But it's been checked in as part
of 128962.  we need that for the account mananger extension for MDN.

about http://bugzilla.mozilla.org/attachment.cgi?id=73377&action=view

jefft, if you apply that patch to you tree, and rebuild, it will enable the pref
UI and the account manager UI for MDN.  right now, it only works for linux and
windows.  shuehan is working on the mac build issues.

note, I'm assuming the MDN extension will always be built.

    
    

    
  


  
Patch with UI integration and fixes with comments from JF and David.
extension/mdn is not build by default. The UI pref integration is not quiet
done yet. Using default prefs and default prefs aren't saved persistently for
the moment.

        Attachment #72934 -
        Attachment is obsolete: true

        Attachment #73165 -
        Attachment is obsolete: true

    
    

    
  


  
This is it. I got all the UIs and features in place. All seems working fine
including using default prefs. Please review. Hope I can start landing them
today.

        Attachment #73377 -
        Attachment is obsolete: true

    
    

    
  
Do we still need these #ifdefs? Does compose depend on mdn still?

+ifdef MDN_ENABLED \
+
		msgmdn \
+endif \
 
	  $(NULL)
 
 CPPSRCS
	= \
Index: mailnews/compose/src/makefile.win
===================================================================
RCS file: /cvsroot/mozilla/mailnews/compose/src/makefile.win,v
retrieving revision 1.51
diff -u -w -r1.51 makefile.win
--- mailnews/compose/src/makefile.win	20 Feb 2002 03:04:40 -0000	1.51
+++ mailnews/compose/src/makefile.win	13 Mar 2002 15:09:21 -0000
@@ -57,6 +57,9 @@
 
	  windowwatcher \
+!if defined(MDN_ENABLED)
+
		msgmdn \
+!endif


    
    

    
  
it seems a little goofy to put those compose utilities in compose instead of
base, if we're really going to be calling them from mdn, compose, and smime. JF,
do you think they should go in base or compose?

Do we really want to return out of the whole routine when this fails? Isn't
there other stuff we want to do after this, no matter if creating this filter
succeeds or fails?

+              rv = newFilter->CreateTerm(getter_AddRefs(term));
+              NS_ENSURE_SUCCESS(rv, rv);
+              rv = term->GetValue(getter_AddRefs(value));
+              NS_ENSURE_SUCCESS(rv, rv);

Do we really want to set the MDN_SENT flag right away, here?

+            msgHdr->OrFlags(MSG_FLAG_MDN_REPORT_SENT, &newFlags);
+            nsCOMPtr<nsIMsgMdnGenerator> mdnGenerator;

Might we want to wait until we got a bit further along? Like seeing if MDN is
installed, even? Or seeing if mdn process succeeded? I know that's no guarantee
that the mdn was sent, since that's asynchronous, but we'd be a little closer.
(similarly, for the pop3 case)

these NS_ENSURE_SUCCESS are dangerous - we're punting on executing whatever
might be left in this routine
+            NS_ENSURE_SUCCESS(rv, rv);
+            rv = mimeHeaders->Initialize(allHeaders, allHeadersSize);
+            NS_ENSURE_SUCCESS(rv, rv);
+            nsCOMPtr <nsIMsgFolder> rootMsgFolder = 
+              do_QueryInterface(m_rootFolder, &rv);
+            NS_ENSURE_SUCCESS(rv, rv);
+            nsMsgKey msgKey;
+            msgHdr->GetMessageKey(&msgKey);
+            


tabs here:
                             void process(in EDisposeType eType, in nsIMsgWindow
aWindow,
+
							 in nsIMsgFolder folder, in nsMsgKey key, 
+
							 in nsIMimeHeaders headers, in boolean autoAction);
+};



    
    

    
  
About those msgComposeUtils function, I don't see smine using any of them!
Now, as mdn need to have access to 2 of them, we could either move them into
base/utils or make them part of an xpcom interface. Jeff choosed the second
option which is more elegant. I am fine with it and seems more logical as you
need those only when you want to use the low level send function available in
msg compose.

    
    

    
  
Jeff, in mdn/build/makefile.win and Makefile.in, you are still doing a static
link with msgcompose! Can you remove that?

    
    

    
  
another thing that confise me: why does nsIMsgMdnGenerator.idl is in
mailnews/base/bublic?

Also, NS_MSGMDNGENERATOR_CONTRACTID and NS_MSGMDNGENERATOR_CID are defined
twice! once in nsIMsgMdnGenerator.idl and once in nsMsgMdnCID.h

    
    

    
  
otherwise, JF, all the mdn code in the core product would need to be #ifdef
MDN_ENABLED, instead of simply checking if we could instantiate the component.
Lot's of #ifdefs are a maintenance nightmare. This way, we could plug in what
ever components support the mdn generator interface, if we put it in base.

    
    

    
  
FYI: I am currently testing MDN on Mac...

    
    

    
  
The UI is not working for me:
-In msg compose, the option menu "Return Receipt" doesn't work at all
-The pref panel "Return Receipt" is blank

However, after modifying manually the prefs, I was able to return a receipt to a
received email.

Does the UI is supposed to work?

    
    

    
  
 correction: only the feedback (no check) is not working with the option menu
item "Return Receipt".

    
    

    
  
Jeff, you need also to update the package file if you want mdn be installed on a
release build: xpinstall/packager/...

    
    

    
  


  
Also, the file mailnews/extensions/mdn/resources/locale/en-us/MANIFEST added in
previous patches is not needed. The file msmdg.properties is already exported
by the jar.nm file.

    
    

    
  


  
JF the UI should be working. I had the similar problem before if I had prefs
with wrong preftype value. Some of the prefs have been changed from Int to
Bool. If things still not working for you could you debug on Mac for me. I
don't have a Mac at home. The xpinstall/packager problem, I would like to
address it in a separate bug. I guess it will be easier to manage if we do
that. Thanks.

        Attachment #73877 -
        Attachment is obsolete: true

    
  
Blocks: 130934

    
    

    
  
all problem reported about UI are gone. I was running a carbon build under
MacOS9. I did a clobber non carbon build and the UI works fine...

    
  
QA Contact: esther → gchan

    
    

    
  
some quick comments:

1)
 
+      if (gMsgCompose.compFields.returnReceipt)
+          document.getElementById("returnReceiptMenu").setAttribute("checked", 
true);
+      else
+          document.getElementById("returnReceiptMenu").setAttribute("checked", 
false);


how about just:

document.getElementById("returnReceiptMenu").setAttribute("checked", 
gMsgCompose.compFields.returnReceipt);

2)


+        if (document.getElementById("returnReceiptMenu").checked)
+        {
+            msgCompFields.returnReceipt = false;
+            document.getElementById("returnReceiptMenu").checked = false;
+        }
+        else
+        {
+            msgCompFields.returnReceipt = true;
+            document.getElementById("returnReceiptMenu").checked = true;
+        }

how about just:

msgCompFields.returnReceipt = !(document.getElementById
("returnReceiptMenu").checked);
document.getElementById("returnReceiptMenu").checked = 
msgCompFields.returnReceipt;

3)

ducarroz, did jefft take care of the cached compose window?  

If a user sets the return receipt and then sends, will the correct return 
receipt state be set when the cached compose window comes back up?


    
    

    
  
Make sure to send mail to robinf to review the strings in 
extensions/mdn/resources/locale/en-US/msgmdn.properties

I'm assuming you got them from 4.x.  If so, I'm sure they are OK.

one comment:

"The sender of the message requested a receipt to be returned. \nDo you wish to 
send one?"

Does using \n work?  How does that look when that prompt shows up in the UI?

    
    

    
  


  
The patch addresses Seth's comments and fixes linux libmsgmdn.so not build
problem. Also, there is a duplicate pref,
mail.server.default.mdn_report_enabled, in mdn.js.

        Attachment #74049 -
        Attachment is obsolete: true

    
    

    
  
Forgot to mention, yes "\n" works fine and the strings are from 4.x.

    
    

    
  
While 4.x may have used that particular UI language for the prompting, having
been at Netscape at that time, I can say as someone who should share in the
blame that UI prompts like that don't make very much sense at all. They were
written at a time when 4.x was for corporate users who were smarty types and
read Infoweek every other day to stay abreast of the latest lingo. That is not
the everyday AOL style user. 

The everyday AOL style user will look at: "The sender of the message requested a
receipt to be returned. Do you wish to send one?" and shake their heads in
puzzlement, as he word 'receipt' as it pertains to email is not in their
vocabulary. Since receipts are associated with payments, they will probably
think AOL is going to bill them for receiving the email message and further
propogate the well known hoax of the post office charging for email messages :-) 

Perhaps this prompt should include a "Help" button that explains what read
receipts are, and how the user could decide to take advantage of that feature
themselves should they desire to do so. 90% of people out there have never heard
of an email receipt. At the very least, a better prompt might be something like:
"The sender requested to be notified when you read this message. Is this ok?"
That is something the AOL-style user might actually understand.  

    
    

    
  
jefft / ducarroz, does this patch play nice with the cached compose window?

    
    

    
  
According to Jeff, the only user-visible alert string is:

+MsgMdnWishToSend=The sender of the message requested a receipt to be returned.
\nDo you wish to send one?

I agree with Angus that while users who request return receipts will know what
they are, recipients may not, so it's best to avoid using that terminology in
the alert.

Suggested text: "The sender of this message has asked to be notified when you
read this message. Do you wish to notify the sender?  [Yes]  [No]

    
    

    
  
A misunderstanding on my part. Most of the other messages in msgmdn.properties
 are the possible messages that the sender can see in the return receipt
notification. I'll review them as well and post comments here.

    
    

    
  
+## Msg Mdn Report strings

I'd like to suggest the following changes:

+MsgMdnDisplayed=Note: This Return Receipt only acknowledges that the message
was displayed on the recipient's computer. There is no guarantee that the
recipient has read or understood the message contents.

+MsgMdnDispatched=The message was either printed, faxed, or forwarded without
being displayed to the recipient. There is no guarantee that the recipient will
read the message at a later time.

+MsgMdnProcessed=The message has been processed by the recipient's mail server
without being displayed to the recipient. There is no guarantee that the
recipient will read the message at a later time.

(others are OK as is)


    
    

    
  
The new +MsgMdnWishToSend & +MsgMdnDisplayed sounds good to me. 

+MsgMdnDispatched may have methods other than printed, faxed or forwarded. 
Fortunately, we don't use it at the moment. 

+MsgMdnProcessed may not be correct. 'cause filtering message into a folder can 
be categrized as processed. It shouldn't be only restricted to the mail server 
action. Maybe adding mail client in it will do. Like this:

+MsgMdnProcessed=The message has been processed either by the recipient's mail 
client or server without being displayed to the recipient. There is no guarantee 
that the recipient will read the message at a later time.

    
    

    
  
Seth, the cached compose window does work. However, your

msgCompFields.returnReceipt = !(document.getElementById
("returnReceiptMenu").checked);
document.getElementById("returnReceiptMenu").checked = 
msgCompFields.returnReceipt;

doesn't work for me. There are some latency problem of setting correct value 
that I couldn't figure out. I am reverting it back to my old code which seems 
work more accurate.

    
    

    
  
Jeff, do you check the menu when we open a compose window if the setting say to
always request a receipt?

    
    

    
  


  
JF, yes, I tested it. The previous patch doesn't work due to a change I made to
ToggleReturnReceipt(). This one should fix the problem. This patch also
contains Robin's suggestion of how we word the mdn message.

        Attachment #74289 -
        Attachment is obsolete: true

    
    

    
  
Jeff, your suggested text for +MsgMdnProcessed
 is fine with me.

    
  

        Attachment #73476 -
        Attachment is obsolete: true

    
    

    
  
With regard to the processed message:

+MsgMdnProcessed=The message has been processed either by the recipient's mail 
client or server without being displayed to the recipient. There is no guarantee 
that the recipient will read the message at a later time.

Stop me if I'm being silly, but surely if the email server processed the email
without it having reached the client, then that means that Moz has not seen the
message yet, so how can Moz send the above message?  By virtue of the fact that
Moz will be sending this message, you can drop the bit about the server having
processed it and thus remove some possible element of confusion:

+MsgMdnProcessed=The message has been processed by the recipient's mail 
client without having being displayed to the recipient. There is no guarantee 
that the recipient will read the message at a later time.

Yes?

    
    

    
  
Yes, agreed.

    
    

    
  
+MsgMdnProcessed=The message has been processed by the recipient's mail 
client without having being displayed to the recipient. There is no guarantee 
that the recipient will read the message at a later time.

should be changed to 
+MsgMdnProcessed=The message has been processed by the recipient's mail 
client without having been displayed to the recipient. There is no guarantee 
that the recipient will read the message at a later time.

That's "having been" as opposed to "having being". Alternatively, drop the 
"having" altogether so it says, "without being displayed"

or, how about:
The message was processed by the recipient's mail client without being 
displayed. There is no guarantee that the message will be read at a later time.




    
    

    
  
If the Return receipt menu item is correctly checked or unchecked when we load
the window (after we get the compose field) then yes, it takes care of the
recycled compose window!

    
    

    
  
We're still doing things like NS_ENSURE_SUCCESS(rv, rv) in places where we
should not be bailing out if sending an MDN fails:

+            nsCOMPtr<nsIMsgMailNewsUrl> 
+                msgUrl(do_QueryInterface(imapUrl, &res));
+            NS_ENSURE_SUCCESS(res,res);
+            nsCOMPtr<nsIMsgWindow> msgWindow;
+
+            mdnGenerator =
+                do_CreateInstance(NS_MSGMDNGENERATOR_CONTRACTID, &res);
+            // NS_ENSURE_SUCCESS(res,res);
+            // To ensure code works w/o MDN enabled
+            if (mdnGenerator)
+            {
+                res = msgUrl->GetMsgWindow(getter_AddRefs(msgWindow));
+                NS_ENSURE_SUCCESS(res, res);
+            
+                res = msgUrl->GetMimeHeaders(getter_AddRefs(mimeHeaders));
+                NS_ENSURE_SUCCESS(res,res);
+
+                mdnGenerator->Process(nsIMsgMdnGenerator::eDisplayed,
+                                      msgWindow, this, uidOfMessage, 
+                                      mimeHeaders, PR_FALSE);
+                msgUrl->SetMimeHeaders(nsnull);
+            }
+        }
         msgHdr->MarkRead(PR_TRUE);

all of those are bad, IMO. I'd still like to mark the message read, even if I
couldn't do some MDN processing. If you really want to use those macros, you
should move out that code into a seperate method.

Similarly, in the local mail case, there's a bunch more of them.

    
    

    
  
I thought I fixed that. I guess going back and forth on Windows and Linux build
screwed me up. :-( I'll make sure that being addressed for next patch. 

    
    

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

    
    

    
  


  
Try again with no NS_ENSURE_SUCCESS calls in nsImapMailFolder.cpp,
nsMailProtocol.cpp & nsParseMailbox.cpp for mdn related processes. I like Tim
Wunder's suggestion of MsgMdnProcessed string so I incorporated it in.

        Attachment #74487 -
        Attachment is obsolete: true

    
    

    
  
With ref to comment #122, yes that was my typo.  I meant to type "having been",
but noticed it after I'd clicked the Commit button.  Oops.

Anyway, the alternative offered by Tim seems fine to me.


    
    

    
  
Agree - the proposed MsgMdnProcessed string text looks good.

    
    

    
  
a few nits, pretty much all style and questions about comments. If you address
these issues, then r=bienvenu. I don't need to see another patch unless Seth
wants another one for his sr.

 - don't need braces here:

+    if (NS_FAILED(rv)) {
+        return;
+    }

or here:
+        if ( ! m_mdnBundle ) {
+            return rv;
+        }

or here:
+    if (name) {
+        rv = m_mdnBundle->GetStringFromName(name, outString);
+    }

or here:
+        if ( ! m_mdnBundle ) {
+            return rv;
+        }

or here:
+    if (name) {
+        rv = m_mdnBundle->FormatStringFromName(name, params, 
+                                               numParams, outString);
+    }

this braces style is inconsistent:
+                switch (m_outsideDomainOp) {

with this
if (a)
{
 ...
}

What is this routine doing?

+PRBool nsMsgMdnGenerator::MailAddrMatch(const char *addr1, const char *addr2)
It's not seeing if the strings are exactly the same so some comments would be
useful.

+void nsMsgMdnGenerator::CreateMdnMsg()
I think it would be trivial to rewrite this without using the evil goto:

+    rv = CreateFirstPart();
+    if (NS_FAILED(rv)) goto done;
+    rv = CreateSecondPart();
+    if (NS_FAILED(rv)) goto done;
+    rv = CreateThirdPart();
+ done:
couldn't this just be:

rv = CreateFirstPart();
if (NS_SUCCEEDED(rv))
{
  rv = CreateSecondPart();
  if (NS_SUCCEEDED(rv))
    rv = CreateThirdPart();
}
presto, no goto.

in nsMsgMdnGenerator::CreateFirstPart(), there are several of these:
+    if (NS_FAILED(rv)) return rv;
we want the return rv to be on a separate line.
similarly, in nsMsgMdnGenerator::CreateThirdPart(), same thing.
and nsMsgMdnGenerator::OutputAllHeaders(), same thing

this comment here: What is it that you're trying to deal with? A comment to that
effect would be helpful.
+    // **** this is disgusting; I don't have a better way to deal with it 

these comments: are they still valid? If not, could you remove them, otherwise,
do we need a bug for this?
+    // *** fix me - what if the send failed do we still want to store the
+    // MDNSent flag
+    StoreImapMDNSentFlag(folder, key); // shouldn't we set the pop3 flag too

here, 
+    PRBool m_reallySendMdn;
+    PRBool m_autoSend;
+    PRBool m_autoAction;
+    nsCOMPtr<nsIMsgFolder> m_folder;
+    nsCOMPtr<nsIMsgIncomingServer> m_server;
+    nsCOMPtr<nsIMimeHeaders> m_headers;
+    nsXPIDLCString m_dntRrt;
+    PRBool m_mdnEnabled;

could you put the bools together? - it saves a little space on some machines and
is a good style to have. I would suggest PRPackedBool but we don't create a lot
of these objects :-)

    
    

    
  
yes, can you provide another patch for me to sr, and to use for test builds?

    
    

    
  


  
This patch addresses David's comments and ready for sr, also for test build.

        Attachment #74894 -
        Attachment is obsolete: true

    
    

    
  
some comments.

1)

I still think this can be simplified.  I'm not sure why my suggestion didn't 
work for you.

+function ToggleReturnReceipt()
+{
+    var msgCompFields = gMsgCompose.compFields;
+    if (msgCompFields)
+    {
+        if (document.getElementById("returnReceiptMenu").checked)
+        {
+            msgCompFields.returnReceipt = false;
+            document.getElementById("returnReceiptMenu").checked = false;
+        }
+        else
+        {
+            document.getElementById("returnReceiptMenu").checked = true;
+            msgCompFields.returnReceipt = true;
+        }

2)

remove (or comment out) the dump

+        dump("retrunReceipt: " + msgCompFields.returnReceipt +"\n");

3)

These are pointers, so NS_ENSURE_ARG_POINTER() would follow our convention.
 
+  NS_ENSURE_ARG(prefix);
+  NS_ENSURE_ARG(identity);
+  NS_ENSURE_ARG(_retval);

4)

can you elaborate here?  does this code not work when you switch identities?  
If so,
we should log a bug for this.
         
+      // ***** fix me!!! when switching identity

5)


+          nsCOMPtr<nsIPref> prefs(do_GetService(NS_PREF_CONTRACTID, &rv));
+          if (NS_SUCCEEDED(rv) && prefs) 

prefs will never be null, if rv is success.  I'd suggest:
because if prefs fail, you are in big trouble.

+          nsCOMPtr<nsIPref> prefs = do_GetService(NS_PREF_CONTRACTID, &rv);
+          NS_ENSURE_SUCCESS(rv,rv);
+          rv = prefs->GetBoolPref("mail.receipt.request_return_receipt_on",
+                                      &requestForReturnReceipt); 
+          rv = prefs->GetIntPref("mail.receipt.request_header_type",
+                                     &receiptType);

6)

Your makefiles look copied and pasted.  why do we require pipnss for the linux 
build, but not the windows one?
Why do we require addrbook?
Index: extensions/mdn/src/makefile.win
 
7)  are there contract ids you can use, instead of defining static CIDs?

+
+static NS_DEFINE_CID(kCMimeConverterCID, NS_MIME_CONVERTER_CID);
+static NS_DEFINE_CID(kHTTPHandlerCID, NS_HTTPPROTOCOLHANDLER_CID);
+static NS_DEFINE_CID(kSmtpServiceCID, NS_SMTPSERVICE_CID);

8)

I don't think you need to cache your own bundle.

a) the string bundle service cache bundles, so if it is frequently used, it 
will be cached
b) I don't think you gain anything by caching it yourself.  you just make the 
code more complicated.

I'd just keep the helper methods (like GetMdnBundleString(), and make them go 
get the bundle every time.)

It should make your code simpler.

9)

+        const char *accountDomain = PL_strchr(m_email.get(), '@');

use strchr instead.


10)

+        // *** fix me
+        // *** what if the message has been filtered to different account

if this is an issue, please log a bug and include the bug number in your 
comment.

11)

if this is an issue, please log a bug and include the bug number in your 
comment.

+        // *********
+        // How are we gona deal with the auto forwarding issues? Some server
+        // didn't bother to add addition header or modify existing header to
+        // thev message when forwarding. They simply copy the exact same
+        // message to another user's mailbox. Some change To: to
+        // Apparently-To: 
+        // *********

12)

do strchr() and strncmp() instead.

+    lt = PL_strchr(addr1, '<');
+    lt = PL_strchr(addr2, '<');
+    end1 = PL_strchr(local1, '>');
+    end2 = PL_strchr(local2, '>');
+    atSign1 = PL_strchr(local1, '@');
+    atSign2 = PL_strchr(local2, '@');
+    else if (PL_strncmp(local1, local2, (atSign1-local1))) // case sensitive


13)

to and cc are XPIDLCStrings, right?  they should not be null.

do:
  to.Length()  &&
or
  (!to.IsEmpty()) &&

instead

same goes for cc, reply_to.

+  // start with a simple check
+  if ((to && PL_strcasestr(to.get(), m_email.get())) || 
+      (cc && PL_strcasestr(cc.get(), m_email.get()))) {
+      return PR_FALSE;
+  }
+
+  if ((reply_to && to && PL_strcasestr(to.get(), reply_to.get())) ||
+      (reply_to && cc && PL_strcasestr(cc.get(), reply_to.get()))) {
+      return PR_FALSE;
+  }

13)

This what if the temp file already exists?  
maybe we left it there because we crashed, or I'm on a unix machine, and 
someone else is creating the same file to /tmp
I think you should be trying to create a unique file in the OS temp directory/

+    nsSpecialSystemDirectory
+        tmpFile(nsSpecialSystemDirectory::OS_TemporaryDirectory); 
+    tmpFile += "mdnmsg";

14)

use strlen() instead.

+    PR_snprintf(tmpBuffer + PL_strlen(tmpBuffer), 100,
+                "%c%02d%02d" CRLF,
+                (gmtoffset >= 0 ? '+' : '-'),
+                ((gmtoffset >= 0 ? gmtoffset : -gmtoffset) / 60),
+                ((gmtoffset >= 0 ? gmtoffset : -gmtoffset) % 60));

15)

do these lossy converts mean that MDN won't work for non-US-ASCII users?

m_charset should be ok, but if localizers localize the properties file, I think 
your
lossy convert will fail.

+        m_email.get(), PR_TRUE, NS_LossyConvertUCS2toASCII(m_charset).get(), 0,
+        PR_smprintf (NS_LossyConvertUCS2toASCII(mdnMsgSentTo).get(), 
+                         NS_LossyConvertUCS2toASCII(receipt_string).get() :
+                         "Return Receipt"), (subject ? subject.get()  : ""));
+        PR_FALSE, NS_LossyConvertUCS2toASCII(m_charset).get(), 0,

16)

is that \r\n correct, for all platforms?  same with CRLF.

+    tmpBuffer = PR_smprintf("Content-Type: multipart/report; \
+report-type=disposition-notification;\r\n\tboundary=\"%s\"" CRLF CRLF,
+                            m_mimeSeparator.get()); 

17)

looks like all the #ifdef DOING_WORD_WRAP code can be removed, right?

Or do we need to implmenet XP_WordWrap()?

18)

you mean PR_FALSE, right?

+            PRBool useCustomPrefs = 0;
+            m_identity->GetBoolAttribute("use_custom_prefs", &useCustomPrefs);

19)

see comment #5

+                    if (NS_SUCCEEDED(rv) && prefBranch)
+                    {
+                        PRBool bVal = PR_FALSE;
+                        prefBranch->GetBoolPref("mail.mdn.report.enabled",
+                                                &bVal);
+                        m_mdnEnabled = bVal;
+                        prefBranch->GetIntPref("mail.mdn.report.not_in_to_cc",
+                                               &m_notInToCcOp);
+                        prefBranch->GetIntPref
("mail.mdn.report.outside_domain",
+                                               &m_outsideDomainOp);
+                        prefBranch->GetIntPref("mail.mdn.report.other",
+                                               &m_otherOp);
+                    }

20)

can this be removed?

+#if 0
+              rv = newFilter->CreateTerm(getter_AddRefs(term));
+              if (NS_SUCCEEDED(rv))
+              {
+                  rv = term->GetValue(getter_AddRefs(value));
+                  if (NS_SUCCEEDED(rv))
+                  {
+                      value->SetAttrib(nsMsgSearchAttrib::OtherHeader);
+                      value->SetStr(NS_LITERAL_STRING("delivery-status").get
());
+                      term->SetAttrib(nsMsgSearchAttrib::OtherHeader);
+                      term->SetOp(nsMsgSearchOp::Contains);
+                      term->SetBooleanAnd(PR_FALSE);
+                      term->SetArbitraryHeader("Content-Type");
+                      term->SetValue(value);
+                      newFilter->AppendTerm(term);
+                  }
+              }
+#endif


21)

can you do do_CreateInstance with the contract ID?  I see from LXR that there 
is no contract ID for this.
now would be a good time to define one, and use it in your code and 
mozilla\mailnews\mime\src\nsStreamConverter.cpp

+                rv = nsComponentManager::CreateInstance(
+                        kCIMimeHeadersCID, nsnull, NS_GET_IID(nsIMimeHeaders),
+                        (void **) getter_AddRefs(mimeHeaders));

22)

see #21

+                    rv = nsComponentManager::CreateInstance(
+                         kCIMimeHeadersCID, nsnull, NS_GET_IID(nsIMimeHeaders),
+                         (void **) getter_AddRefs(mimeHeaders));

23)

you know, there seems to be some duplicated code to create a new filter, and 
set the value and the terms.
why not add all that code to one place (maybe the mdn service?) and then from 
both imap and pop, 
get the mdn service, and create the filter.  can you look into that?

24)

again, contractid instead of static CID.

+        nsCOMPtr<nsIMimeHeaders> mimeHeaders;
+        rv = nsComponentManager::CreateInstance(
+            kCIMimeHeadersCID, nsnull, NS_GET_IID(nsIMimeHeaders),
+            (void **) getter_AddRefs(mimeHeaders));
+        NS_ENSURE_SUCCESS(rv,rv);
+        mimeHeaders->Initialize(allheaders, allheadersize);
+        msgurl->SetMimeHeaders(mimeHeaders);
+    }
+    return NS_OK;
 }
 
25)

something to keep in mind, idl should be interCaps.

So, for this method, add it as addAllHeaders().
 
+    void    AddAllHeaders([const] in string allheaders, [const] in long 
allheadersize);


also, I don't think the const is nececcessary.

26)


+	string extractHeader ([const] in string headerName, in boolean
+                          getAllOfThem);
+    string getAllHeaders (out long allHeadersSize);

isn't all headersSize just the length of the string?  Or are there null bytes 
in there?

If so, this could be:

readonly attribute string allHeaders;

And your caller, can just call rv = foo->GetAllHeaders(getter_Copies(str));
and do str.Length() to get the size.

Or, if there is null bytes, then I don't think it's save to use a string like 
this.  you'll want to use a range of bytes.

27)

double check, I don't think you require all these.

Index: extensions/mdn/build/Makefile.in

28)

you need to static link in xpcom and js32?

+LLIBS=										
	\
+			$(LLIBS)						
	\
+			$(DIST)\lib\xpcom.lib				\
+			$(DIST)\lib\msgbsutl.lib	\
+			$(DIST)\lib\js32$(VERSION_NUMBER).lib	\
+			$(LIBNSPR)						
	\
+			$(NULL)

29)

I don't think these are necessary, and have been removed.

Index: extensions/mdn/resources/content/Makefile.in
Index: extensions/mdn/resources/content/makefile.win

30)

this is already checked in.

update you tree before creating a new patch.

Index: extensions/mdn/resources/content/mdn.js
===================================================================
RCS file: /cvsroot/mozilla/mailnews/extensions/mdn/resources/content/mdn.js,v
retrieving revision 1.2
diff -u -w -r1.2 mdn.js
--- extensions/mdn/resources/content/mdn.js	13 Mar 2002 01:41:08 -0000
	1.2
+++ extensions/mdn/resources/content/mdn.js	20 Mar 2002 02:16:52 -0000
@@ -15,5 +15,3 @@
 pref("mail.server.default.mdn_other", 2); 
 
 pref("mail.identity.default.request_receipt_header_type", 0); // return 
receipt header type - 0: MDN-DNT 1: RRT 2: Both
-
-pref("mail.server.default.mdn_report_enabled", true);

    
    

    
  


  
4) There is a bug logged for switching identity problem. bug 129418

6) Indeed I was copying from smime. I'll play arround to remove
   unneeded modules.

8) I guess you meant not to use static member variable
   to cache the string bundle for the class. I'll make the change to
   not using static member variable. I don't think you would want to call
   do_GetServer() then CreateBundle() each time you call
   GetMdnBundleString(), right?

11) This isn't an issue because it's out of our control. We cannot do
    anything about it.

15) Don't know how to deal with this. MDN is sort of designed for RFC
    822/2822 message. All email addr-spec has to be US-ASCII except
    the comment part of the addr-spec. In another word, the real
    email address has to be only contains US-ASCII. The pretty name
    can be double bytes or any kind of language string. So, lossy
    convert should be fine. 

16) Yes, \r\n is correct. You have to generate a canonical RFC
    822/2822 message prior deliver to an SMTP server. The SMTP service
    double check to make sure the outgoing message has each line ended
    with \r\n.

17) We can remove that. Nowadays almost all MTA can handle message
    lines more than 76 characters. There shouldn't have any legacy MTA
    servers in the world to chop the long message lines. Cross my fingers.

27) Don't know what you mean about this. The Makefile.in is needed but
    a lot of unneeded modules have been removed.

28) xpcom.lib is needed but not js32 lib.

29) The makefiles are needed to update the chrome jar files. It fails
    to build if removed. It does need to place mdn.js in the
    defaults/pref directory. 

30) No, the fix is not in. There are two entries of
    pref("mail.server.default.mdn_report_enabled", ...); in mdn.js one
    in the middle with value set to false and the other at the end
    with the value set to true.

        Attachment #75109 -
        Attachment is obsolete: true

    
    

    
  
> 4) There is a bug logged for switching identity problem. bug 129418

can you include the bug number in the comment around that code?

> 6) Indeed I was copying from smime. I'll play arround to remove
>   unneeded modules.

does your last patch include the cleaned up version of the makefiles?

>8) I guess you meant not to use static member variable
>   to cache the string bundle for the class. I'll make the change to
>   not using static member variable. I don't think you would want to call
>   do_GetServer() then CreateBundle() each time you call
>   GetMdnBundleString(), right?

yes, calling getservice and then createbundle won't be that expensive because:

1)  the string bundle service caches strings bundles
2)  your string bundle calls are going to be few, and far between.

I'm suggestion that you don't need to cache the bundle.

allow the string bundle service to cache for you.

add something like this:

nsresult nsMsgMdnGenerator::GetMdnStringBundle(nsIStringBundle **aBundle)
{
  NS_ENSURE_ARG_POINTER(aBundle);

  nsresult rv;
  nsCOMPtr<nsIStringBundleService>
   bundleService(do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv));

  NS_ENSURE_SUCCESS(rv,rv);

  rv = bundleService->CreateBundle(MDN_STRINGBUNDLE_URL, aBundle);
  NS_ENSURE_SUCCESS(rv,rv);
  return rv;
}

and in your methods:


nsresult nsMsgMdnGenerator::GetMdnBundleString(const PRUnichar *name,
                                               PRUnichar **outString)
{
  NS_ENSURE_ARG_POINTER(name);
  NS_ENSURE_ARG_POINTER(outString);

  nsresult rv;
  nsCOMPtr <nsIStringBundle> bundle;
  rv = GetMdnStringBundle(getter_AddRefs(bundle));
  NS_ENSURE_SUCCESS(rv,rv);

  rv = bundle->GetStringFromName(name, outString);
  NS_ENSURE_SUCCESS(rv,rv);
  return rv;
}


    
    

    
  
I've applied jefft's patch locally.

I've fixed the string bundle code, and fixed some build issues.

for example, mailnews/base/public/nsIMsgMdnGenerator.idl was include 
nsIMimeHeader.idl, which won't be there for a clobber build, because mime is 
built after base.

That's easily fixed by doing "interface nsIMimeHeaders;"

I'm looking into removing some unnecessary makefiles in the mdn extension.

once I work that out, I'll test the code, do a final review.

Then I'll land the new files (that are not part of the build) so that I can 
generate test builds for QA.

I'll report back soon.

    
    

    
  
The last patch (http://bugzilla.mozilla.org/attachment.cgi?id=75724&action=view)

is what you'd need to apply to a fresh tree.  jefft, when you update you are 
going to get conflicts.  I recommend updating and moving files out the way.

jefft, here are some changes that are either in the patch, or in the files I 
checked in:

1) 

string bundle changes made to nsMsgMdnGenerator.cpp

2)

changes to nsIMsgMdnGenerator.idl so it would build on clobber builds

3) 

removed static NS_IMIMEHEADERS_CID, now using the contract ID.

see nsStreamConverter.cpp and nsMimeBaseEmitter.cpp

4) 

the follow files have been checked in because they are new, or not part of the 
default build.

This make it easier for me for us to do the mac build changes, or to spin up 
test builds.

mozilla/mailnews/base/public/nsIMsgMdnGenerator.idl
mozilla/mailnews/compose/public/nsIMsgCompUtils.idl
mozilla/mailnews/extensions/mdn/src/
mozilla/mailnews/extensions/mdn/src/Makefile.in 
mozilla/mailnews/extensions/mdn/src/makefile.win 
mozilla/mailnews/extensions/mdn/src/nsMsgMdnGenerator.cpp 
mozilla/mailnews/extensions/mdn/src/nsMsgMdnGenerator.h 
mozilla/mailnews/extensions/mdn/resources/locale/en-US/msgmdn.properties
mozilla/mailnews/extensions/mdn/build/Makefile.in 
mozilla/mailnews/extensions/mdn/build/makefile.win 
mozilla/mailnews/extensions/mdn/build/nsMsgMdnCID.h 
mozilla/mailnews/extensions/mdn/build/nsMsgMdnFactory.cpp  
mozilla/mailnews/extensions/mdn/Makefile.in 
mozilla/mailnews/extensions/mdn/jar.mn 
mozilla/mailnews/extensions/mdn/makefile.win 

When the final review happens, we'll need to review a patch and those added 
files.

5)

there were some extra makefile.win Makefile.in and MANIFEST files under 
mozilla/mailnews/extensions/mdn 
that were part of jefft's patch that are not required, so I removed them.


test notes:

1)

I've been using the code on my IMAP account.  it plays nice with the cached 
compose window.
and the per account overrides seem to be working.

Only one problem so far:  my return receipt always ends up in my sent folder.  
I can't seem to get my rr in my inbox.

2)

I'm going to create an optimized linux and windows builds for gchan to test.

I haven't tried building on mac.  jefft, you might need ducarroz to help you 
with that.  (update his tree, and apply that last patch.)



    
    

    
  
Alright. Cool. Thanks much, Seth.

    
    

    
  
I figured out the problem of not able to place rr in Inbox. If your start with
placing rr in Sent folder an MDN message filter would be added to the filter
list to help the incorporation process. It stays in the filter list until the
session ends. So, when switching incorporating rr to Inbox we have to either
disable the MDN filter or remove it from the filter list. I'll work on the fix.

    
    

    
  
update:  I've clobber built and tested this on linux.  It looks good.

the Makefile.in's that I removed from jefft's patch were not necessary.

next step:  provide some builds for QA.

I'll wait until jefft fixes the
http://bugzilla.mozilla.org/show_bug.cgi?id=16241#c140




    
    

    
  
The per-account overrides should default to "use global settings".

most people will go right to Edit | Prefs, not the account settings.

So they'll think they're affecting the MDN prefs, but they won't be, if the
default is not "use global settings".

I've made the change and checked it in.

Here's the fix:

Index: mdn.js
===================================================================
RCS file: /cvsroot/mozilla/mailnews/extensions/mdn/resources/content/mdn.js,v
retrieving revision 1.2
diff -r1.2 mdn.js
5c5
< pref("mail.identity.default.use_custom_prefs", true);            // false: Use
global true: Use custom
---
> pref("mail.identity.default.use_custom_prefs", false);            // false:
Use global true: Use custom

    
    

    
  


  
This is it for sure this time. It took me a while to verify it work due to a
inconsistent state in my mork database. I aborted my debugging session in the
middle of updating mork database. Everything should work fine now. JF, could
you help me with a test Mac build for Gary to test with? Thanks much.

        Attachment #75582 -
        Attachment is obsolete: true

        Attachment #75724 -
        Attachment is obsolete: true

    
    

    
  
one thing we forgot is to update the packager files.

to package mdn-service.js, msgmdn.dll, etc.

look in mozilla\xpinstall\packager\*



    
    

    
  
xpinstall/packager files are addressed in bug 130934.

    
    

    
  
on a new profile, I'm not crashing.  but on my existing profile I'm crashing in 
a security library 

SOFTOKN3.DLL
from mozilla/security/nss/lib/softoken

I've put some windows, optimized, mozilla bits in

/u/sspitzer/mdn.zip

for gchan to test.  linux coming later today.

    
    

    
  
quick heads up, still testing it. But everything seems
fine w/test build Seth provided. Seems to work fine.
No crashes or anything unusual.

I'm still trying to do more detailed testing (different
scenarios, mail acts, clients, etc..) but it looks good
so far.

    
    

    
  
sorry for the delay, gary.

the linux, optimized mozilla mdn bits are at

/u/sspitzer/mdn-linux.zip

    
    

    
  


  
the only other change needed is to mailnews-ns.js in the commercial tree.

I added:

pref("imap.aol.canHaveFilters", false);

this change makes it so:
1)  we don't attempt to filter return receipts to the sent folder for AOL
accounts
2)  the "create message filter" link is hidden in account central for AOL
accounts
3)  aol accounts don't show in the server picker in the message filter dialog

    
    

    
  
Comment on attachment 76265 [details] [diff] [review]
changes since last patch to handle AOL mdn issues

talked to bienvenu over aim, going to do something a little different patch on
the way.

        Attachment #76265 -
        Attachment is obsolete: true

    
    

    
  


  
ok, this is a patch that contains the diff between the last jefft patch and
what I've got in my tree.

filters on aol accounts are OK, just not certain types.  (so those bugscape
bugs might turn into wontfix).	note, the filter UI will continue to show up
for AOL servers, as it should.

we have to check if the current imap server allows us to file messages into it.

aol imap servers don't, and we already have code to check that property.

this change makes it so we check that, and if we can't do a file, we don't
create the mdn filter to file the RRs to the sent folder.  (no matter what the
pref UI says).

I've also removed an unused setter.

    
    

    
  
windows, optimized, commercial mdn bits:

/u/sspitzer/nsmdn.zip

    
    

    
  
there a couple of kinks I'm trying to work out.

specifically, there are issues with custom headers, the filter UI and writing 
out the filter to disk.

to see this, filter your RRs to your sent folder and then go edit your filters.

among other problems, the internal filter shows up, will get written to disk 
and we pop up the custom header dialog.

I'm talking with bienvenu over aim about this.

    
    

    
  


  
is jefft's last patch, address the AOL issue, contains some clean up.

what's left before a final review is filter / custom header issue.

I'll work on that tonight, I hope to have something for tomorrow.

        Attachment #75795 -
        Attachment is obsolete: true

        Attachment #76281 -
        Attachment is obsolete: true

    
    

    
  
Oops, missed that. Sounds to me the right thing to do would be to hide the MDN 
filter when bring up the Edit Message Filter window.

    
    

    
  


  
my only remaining issue is how we use ::OtherHeader.

I'll discuss with jefft / bienvenu tomorrow.

        Attachment #76343 -
        Attachment is obsolete: true

    
    

    
  
some nits:

+    m_temporary(PR_FALSE),
     m_type(1),
     m_filterList(nsnull)

m_temporary should be initialized after m_type and before m_filterList to avoid
linux warnings, I believe.

+        PRBool isTemporary;
+        rv = filter->GetTemporary(&isTemporary);
+        NS_ENSURE_SUCCESS(rv,rv);
+        if (isTemporary)
+          continue;

NS_ENSURE_SUCCESS is not needed here since the accessor never fails, and if
anything did go wrong, you couldn't edit your filters. 

Has AddMdnFilter always been there, and on the nsIMsgFolder? I would put it on
the server, since it's really more of a server level thing, and nsIMsgFolder is
so huge already.

There seem to be lots of tabs in nsParseMailbox.cpp.

    
    

    
  
working on david's comments now...

    
    

    
  
Comment on attachment 76448 [details] [diff] [review]
patch, more cleanup, testing it now, ready for review.

sr=bienvenu, thx,Seth.

        Attachment #76448 -
        Flags: superreview+

    
    

    
  
Cool. Well done.

1)
\ No newline at end of file -- for nsMsgIncomingServer.cpp


    
    

    
  
Comment on attachment 76448 [details] [diff] [review]
patch, more cleanup, testing it now, ready for review.

r=sspitzer on the code in this patch that is jefft's, and the jefft code that
is in the tree, but not part of the build.

        Attachment #76448 -
        Flags: review+

    
    

    
  
Comment on attachment 74045 [details] [diff] [review]
Complement patch for Mac (mdn disabled)

r=sspitzer

ducarroz is going to change mdn to 1 and then land this for us, once the tree
is open, and we get a=.

        Attachment #74045 -
        Flags: review+

    
    

    
  
r=jefft on seth clean up and fixes other than mine. thanks seth.

    
    

    
  
Comment on attachment 74045 [details] [diff] [review]
Complement patch for Mac (mdn disabled)

sr=bienvenu

        Attachment #74045 -
        Flags: superreview+

    
    

    
  
Comment on attachment 76471 [details] [diff] [review]
installer fixes

r=ducarroz, sr=sspitzer

        Attachment #76471 -
        Flags: superreview+

        Attachment #76471 -
        Flags: review+

    
    

    
  
I see judging by the comments it's going to be landing soon.

Give my quick summary on test builds:

I tested all 4 builds (windows, mac,linux, and commercial windows)
that Seth and Ducarroz provided.

Seth, I couldn't use the commercial build to test aol/webmail.
It looked like commercial build but behaved like mozilla build.
(couldn't login to webmail or send mail from aol)
I assume no big deal based on your comments in this bug already.

The other 3 builds seemed to be fine. Mail seemed fine. able to
compose,reply,forw,read, delete, etc..

I was able to use global pref or individual MDN prefs and they
work as expected. Tried imap/pop mail accounts and no problems.

Tried overiding return recepits by unchecking the setting
in a individual mesg and it worked.

Tried offline, doing a send later/send unsent mesgs, using a
simple filter and MDN worked as expected.

Return receipt mesg contains general disclaimer, mdn header?,
and header of the message.

Please let me know if I missed anything/something you want me to
check.




    
  
Whiteboard: [P1] → [adt2][P1]

    
    

    
  
> Seth, I couldn't use the commercial build to test aol/webmail.
> It looked like commercial build but behaved like mozilla build.
> (couldn't login to webmail or send mail from aol)

I'll try to get you another one.  I may have packaged it incorrectly.

    
    

    
  
note to bienvenu / jefft, I've fixed the tabs in my local tree.

seeking approval with drivers now.

    
    

    
  
Comment on attachment 74045 [details] [diff] [review]
Complement patch for Mac (mdn disabled)

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

        Attachment #74045 -
        Flags: approval+

    
    

    
  
Comment on attachment 76448 [details] [diff] [review]
patch, more cleanup, testing it now, ready for review.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

        Attachment #76448 -
        Flags: approval+

    
    

    
  
Comment on attachment 76471 [details] [diff] [review]
installer fixes

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

        Attachment #76471 -
        Flags: approval+

    
    

    
  
Marking it fixed. Seth and JF just landed the fix. Thanks so much. 
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 22 years ago
Resolution: --- → FIXED

    
    

    
  
No, thank YOU for spending time on this. Please tell me where to send the free
beer/pizza coupons... 

    
    

    
  
Using Mac OS X build 2002040108 under OS X 10.1.3

When I send an email requesting a return receipt, it seems to request one ( I
assume that that is what the "Disposition-Notification-To" field in the headers
is about.

However when I receive the message with "Allow return receipts to be sent" set
to "Always" no receipt seems to be getting sent.  If I specify to "Ask Me" in
the preferences, I do not get asked and no receipt is sent.

I have tried this in the global prefs with my account prefs set to "Use global
prefs" and I have tried it with setting the prefs directly in the local account
preferences; in all cases I get the same results.

It also does not seem to matter whether or not I specify to leave the receipt in
the Inbox or if I have it put in Sent

    
    

    
  
Using April 02 commercial trunk build: is it a bug that the Return Receipts item
appears in the Options menu in the Mail Compose window when I'm replying to a
message in a newsgroup account?

    
    

    
  
Not sure, you can address a newsgroup message to an e-mail recipient as well,
but only the e-mail recipient should get the MDN request, right, Jeff?

    
    

    
  
Yes, the message goes to the newsgroupd shall not have the
Disposition-Notification-To header in it.

    
    

    
  
Using 2002040203 commercial trunk on  NT 4.0.

-Tested sending a mesg to a newsgroup w/MDN preferences on requesting
 a return receipt. 
 Result: Mesg got posted to the newsgroup. I read the mesg. No window
         asking me to send a read receipt. I went back into the original
         email account and there was no read receipt in my inbox/sent folder.

         I looked at the header of my mesg that I posted in the newsgroup
         and it shows this: Disposition-Notification-to: g<g@email.com>

         That a bug?

-Tested Sending a Mail mesg w/Return Receipt that was addressed to mail 
user and a newsgroup
   Result: The Mail receipient was requested to send a return receipt,
           I got the return receipt, and mesg was posted to the newsgrp
           The return receipt mesg looked fine (contained Reporting
           UA,Final-Recepient,Orginal Mesg id, and Disposition:
           manual-action/MDN-sent-manually; displayed)


              
                

    
    

    
  
reading a news message in a newsgroup should NOT result in a return receipt -
MDN should be completely turned off in news. If I understand you correctly, the
current behaviour is correct.

    
    

    
  
I just tested again using OS X build 2002040308 and it is still not sending
reciepts.

Has anyone else tested using OS X builds?

    
    

    
  
Mark,
If you see a bug, can you file a new one please?
This bug is not meant to track all mdn bugs.


FYI: i tested commercial trunk 2002040203 on os 10.1.3 using imap mail acnt
and it works for me. I requested a return receipt and I got one
back. I also was prompted to send a return receipt also.

File a New bug and we'll try to answer your questions in that one. thanks.





    
    

    
  
Gary, message goes to the newsground should *not* have the 
Disposition-Notification-To: header in it. This definitely a bug. Please log a 
new bug. Thanks.

    
    

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

    
    

    
  
Verified FIXED.

This feature has been implemented per the spec at
http://www.mozilla.org/mailnews/specs/receipts/, and additional bugs about
functionality/UI have already been filed.

Builds: 2002-09-23-08, All OSs (RedHat 7.3, Windows 2000, Mac OS 9.2.2 / Mac OS
X 10.1.5).

Note that there were several bugs (fixed by Jeff Tsai and Shuehan Liang) that
were already fixed, so this bug's scope is basically 'it's in and works, albeit
with outstanding bugs'.
URL: http://www.mozilla.org/mailnews/specs...
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: