Closed Bug 671673 Opened 10 years ago Closed 9 years ago

mailnews.sendInBackground tries to send in background even when working offline

Categories

(MailNews Core :: Backend, defect)

All
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 8.0

People

(Reporter: jik, Assigned: Bienvenu)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.112 Safari/534.30

Steps to reproduce:

Enabled mailnews.sendInBackground, then enabled "Work Offline", then copied a message into the local Outbox from my add-on, Send Later 3.


Actual results:

Immediately after copying the message into the Outbox, nsMsgSendLater.cpp tried to send it even though I was offline, and the send request hung and continued to be hung even after I went back online.

You can reproduce this without my add-on... enable Work Offline, open a compose window and prepare a test message, open the javascript debugger and focus on the compose window, and evaluate goDoCommand('cmd_sendLater'). It shouldn't start trying to send the message after dropping it into the outbox, but it does.


Expected results:

The code in nsMsgSendLater.cpp should be smart enough not to try to send unsent messages in the background when the user is in offline mode.

Oddly, if I deposit a message directly into the Outbox by clicking the Send Later button, the automatic background send from nsMsgSendLater.cpp doesn't kick off. I can't figure out why this is.
iirc send in background adds a folder listener onto the outbox and looks for a flag either in the message header or database in working out if to send the message or not.

Probably just need to add an offline check in that listener function.
Component: General → Backend
Product: Thunderbird → MailNews Core
QA Contact: general → backend
Attached patch proposed fixSplinter Review
Jonathan, you might as well review this since you're setup to reproduce the issue.
Assignee: nobody → dbienvenu
Attachment #551079 - Flags: review?
Attachment #551079 - Flags: review? → review?(jik)
(In reply to David :Bienvenu from comment #2)
> Created attachment 551079 [details] [diff] [review] [diff] [details] [review]
> proposed fix
> 
> Jonathan, you might as well review this since you're setup to reproduce the
> issue.

Hi David,

I saw some weird behavior when testing this fix which I can't explain.

I put the fix in my source tree, recompiled TB, started it up, set mailnews.sendInBackground to true, and send a message. I was working online, so the message should have been sent in the background immediately, but it wasn't -- it stayed in my Outbox without being sent.

I tried several more times and the same thing happened every time.

Then I recompiled TB without your fix, and that particular problem went away, i.e., messages were sent immediately in the background from my Outbox after I clicked Send.

Now here's where it gets weird... I put the exact same fix back and compiled again with it, and repeated the same test, and this time the problem did not show up, i.e., when online the message was sent in the background from Outbox immediately as expected.

I can't understand why I saw this issue the first time but not the second, but I have a theory... I recall noticing the first time that TB was in the process of refreshing all of its folders and downloading messages, since I had just started it up and there were several folder updates waiting to be done. Is it possible that WeAreOffline() returns false when the initial folder refresh is not yet finished when TB starts up?

If so, then I think this would be a problem with your patch.

Any thoughts?
No, WeAreOffline() simply checks the network io service offline setting - it has nothing to do with any mailnews state.

I can't explain the failure you saw, but kinda I doubt it has anything to do with my patch. Did you check the error console when the send in background was failing?
Yes, I did check the error console, and there wasn't anything relevant. I'll try a few more times to see if I can reproduce the issue.
Comment on attachment 551079 [details] [diff] [review]
proposed fix

I can't reproduce the issue I mentioned earlier. Must have been pilot error, and in any case I can't see how David's change would cause that issue, so I'm going to r=me this.
Attachment #551079 - Flags: review?(jik) → review+
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Checked in: http://hg.mozilla.org/comm-central/rev/912f956bb78f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 8.0
You need to log in before you can comment on or make changes to this bug.