Closed Bug 477680 Opened 11 years ago Closed 11 years ago

Improve the nsIMsgSendLater interfaces and tidy a few bits of code.


(MailNews Core :: Networking: SMTP, defect)

Not set


(Not tracked)

Thunderbird 3.0b2


(Reporter: standard8, Assigned: standard8)




(1 file, 1 obsolete file)

Attached patch The fix (obsolete) — Splinter Review
As part of the work to send messages later, I need to get sensible status feedback. Due to the wonderful way mailnews passes progress to only nsIMsgStatusFeedback instances, I'm going to need to hook one of those into nsIMsgSendLater to get what I want in the short time (beta 2) at least.

I could see a period later of clearing up the listener/feedback options so that we have a much clearer distinction between UI and backend.

This patch:
- Drops the nsIMsgWindow argument to sendUnsentMessages and replaces it by a function to set nsIMsgStatusFeedback directly.
- Drops the redundant OnStatus callback, if this gets replaced, I very much doubt it will be that function prototype.
- Cleans up some of the documentation
- Tidies up SendOperationListener a little (nsresult -> NS_IMETHODIMP and set nsMsgSendLater via the constructor not a separate function).
Attachment #361346 - Flags: superreview?(neil)
Attachment #361346 - Flags: review?(bienvenu)
Attachment #361346 - Flags: review?(bienvenu) → review+
Comment on attachment 361346 [details] [diff] [review]
The fix

>diff --git a/mailnews/base/resources/content/mailWindowOverlay.js b/mailnews/base/resources/content/mailWindowOverlay.js
>--- a/mailnews/base/resources/content/mailWindowOverlay.js
>+++ b/mailnews/base/resources/content/mailWindowOverlay.js
>@@ -2095,7 +2095,8 @@ function SendUnsentMessages()
>       if(msgFolder) {
>         numMessages = msgFolder.getTotalMessages(false /* include subfolders */);
>         if(numMessages > 0) {
>-          msgSendlater.sendUnsentMessages(currentIdentity, msgWindow);
>+          msgSendlater.setStatusFeedback(msgWindow.statusFeedback);
I don't know what jminta has done to TB but currently suite has a global var statusFeedback for this, so you don't need to get it from the msgWindow.

>+    nsCOMPtr<nsIMsgStatusFeedback> feedback;
>+    if (m_window)
>+      m_window->GetStatusFeedback(getter_AddRefs(feedback));
>+    if (feedback)
>+      pMsgSendLater->SetStatusFeedback(feedback);
Can't you use m_StatusFeedback instead?

>+  void setStatusFeedback(in nsIMsgStatusFeedback aFeedback);
Nit: I'd prefer this to be an attribute.

>+SendOperationListener::SendOperationListener(nsMsgSendLater *aSendLater) 
> { 
>-  mSendLater = nsnull;
>+  mSendLater = aSendLater;
Nit: as you're changing this anyway, the constructor form would be better.
Attached patch The fix v2Splinter Review
Revised as per comments.
Attachment #361346 - Attachment is obsolete: true
Attachment #361527 - Flags: superreview?(neil)
Attachment #361527 - Flags: review+
Attachment #361346 - Flags: superreview?(neil)
Attachment #361527 - Flags: superreview?(neil) → superreview+
Checked in:
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.