Closed Bug 471695 Opened 15 years ago Closed 15 years ago

Select Offline -> Download / Sync Now doesn't go offline (not downloading all emails)

Categories

(MailNews Core :: Backend, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: gkw, Assigned: Bienvenu)

References

Details

Attachments

(2 files, 1 obsolete file)

Select Offline -> Download / Sync Now doesn't go offline on Mac latest nightly, Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20081231 Lightning/1.0pre Shredder/3.0b2pre

It downloads all files but the icon on the bottom left doesn't turn to an offline one.
Flags: blocking-thunderbird3?
also doesn't go offline via download /sync in windows 20081230. i can't tell what, if anything is downloaded - progress indicator just spins and status line cites accessing first non-special folder.

but "work offline" works straight off.
OS: Mac OS X → All
Hardware: x86 → All
Just to clarify:

- if I download all files then it won't go offline.
- if I don't download all files, it goes offline ok.
happens at least as far back as 20081209, which I see on my vista box that hasn't had anything newer installed than 20081209.

in contrast to comment 2, for me offline>work offline works regardless
If i uncheck the mail/news/send messages checkbox but do check the 'go offline when done', then it _does_ go offline.

if i check the mail checkbox, then it seems to never end downloading stuff, but I'm not sure if that's just because there's a lot to do.
Figuring this out seems blocking.  We probably want to wait until the current offline/osx bug is fixed in gecko and we flip the switch back there, to avoid confusing things further.
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b3
Putting in mailnews core, as this seems likely to be backend related.  Since Standard8 has been keeping up with the various OS X bugs, reassigning to him.
Severity: critical → major
Component: Mail Window Front End → Backend
Product: Thunderbird → MailNews Core
QA Contact: front-end → backend
Assignee: nobody → bugzilla
Erm, guys, this is NOT a bug, this is how it is supposed to work for years!
I'm using "Offline -> Download / Sync Now" to sync my (locally stored) newsgroups with the server and I'd pretty pissed if MailNews would put me into offline state after that! I still need to be online to continue getting mail/rss, etc. pp...
(In reply to comment #7)
> Erm, guys, this is NOT a bug, this is how it is supposed to work for years!
> I'm using "Offline -> Download / Sync Now" to sync my (locally stored)
> newsgroups with the server and I'd pretty pissed if MailNews would put me into
> offline state after that! I still need to be online to continue getting
> mail/rss, etc. pp...

I'm pretty sure I got confused by this if that's the case - perhaps a naming thing?
> I'm pretty sure I got confused by this if that's the case - perhaps a naming
> thing?

Probably. 
"Offline -> Work Offline" goes offline, but Download/Sync and Get Flagged/Starred/Selected shouldn't.
After all, Work Offline will trigger Download/Sync anyway, if you have configured your account's offline settings that way.
(In reply to comment #9)
> > I'm pretty sure I got confused by this if that's the case - perhaps a naming
> > thing?
> 
> Probably. 

Ok, here's what I think this bug is really about:

1) "Offline -> Work Offline" and choose not to download
--> goes offline correctly.
2) "Offline -> Work Offline" and choose to download
--> downloads but never goes into offline mode.
3) "Offline -> Download/Sync Now" get a dialog, unselect "Work Offline once download is completed"
--> goes offline corectly.
4) "Offline -> Download/Sync Now" get a dialog, select "Work Offline once download is completed"
--> downloads but never goes into ofline mode.

I know for sure that 2 is definitely an issue, I expect through association and this bug that 4 is an issue as well.
Umm, 2) shows a progress indicator saying it downloads, but for me doesn't seem to do anything else.
> 3) "Offline -> Download/Sync Now" get a dialog, unselect "Work Offline once
> download is completed"
> --> goes offline corectly.

"Goes offline correctly" in the sense that it does NOT go offline, because you unchecked the box?
(In reply to comment #11)
> Umm, 2) shows a progress indicator saying it downloads, but for me doesn't seem
> to do anything else.

Do you mean doesn't download messages, or doesn't go offline? If you mean doesn't go offline, that's what I said.

(In reply to comment #12)
> > 3) "Offline -> Download/Sync Now" get a dialog, unselect "Work Offline once
> > download is completed"
> > --> goes offline corectly.
> 
> "Goes offline correctly" in the sense that it does NOT go offline, because you
> unchecked the box?

Opps, yeah, I wouldn't expect that to go offline.
(In reply to comment #13)
> (In reply to comment #11)
> > Umm, 2) shows a progress indicator saying it downloads, but for me doesn't seem
> > to do anything else.
> 
> Do you mean doesn't download messages, or doesn't go offline? If you mean
> doesn't go offline, that's what I said.

Well, it's hard to get a situation where there are messages left to download. There is no download counter or something like this. It just shows the moving activity indicator indefinitely.
Historically, the failure to go offline after the download is caused by an error somewhere in the download process - the download doesn't complete, so we error out, and don't get to the code that takes us offline. It's generally profile-specific, e.g., some account we don't like that makes us error out in the iteration process.
(In reply to comment #15)
> Historically, the failure to go offline after the download is caused by an
> error somewhere in the download process

AFAICT history doesn't apply here. What I've got so far:

1) Select Offline -> Work Offline
2) Say yes to download messages for offline use.
3) Offline service does its Get New Mail cycle.
4) Offline service starts the first of the sync on the folders:

-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:ProcessCurrentURL: entering
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:ProcessCurrentURL:imap://user@hostbox/select%3E.INBOX:  = currentUrl
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:SendData: 16 noop
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=22 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: 16 OK NOOP completed
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:SendData: 17 getquotaroot "INBOX"
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=28 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: * QUOTAROOT "INBOX" "ROOT"
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=16 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: * QUOTA "ROOT"
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=24 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: 17 OK GETQUOTAROOT Ok.
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:SendData: 18 UID fetch 405:* (FLAGS)
-1337147392[1cfd7910]: ReadNextLine [stream=1cfd81cc nb=24 needmore=0]
-1337147392[1cfd7910]: e19c00:imap.plus.net:S-INBOX:CreateNewLineFromSocket: 18 OK FETCH completed.

and stops.

A 2.x build continues with: -1608014048[1109dc0]: offline imap url succeeded :imap://standard8+moztest@imap.plus.net:143/select>.INBOX

As far as I can tell, the OnStopRunningURL callback isn't making it back to the offline service for so it doesn't continue.

As this is now into imap territory I think David will probably spot the error faster than me, so reassigning to him, hope that's ok.
Assignee: bugzilla → bienvenu
(In reply to comment #16)
> (In reply to comment #15)
> > Historically, the failure to go offline after the download is caused by an
> > error somewhere in the download process
> 
> AFAICT history doesn't apply here. What I've got so far:

Not getting the on stop running url is an error in the download process :-) that's definitely been an issue in the past, and I should have called it out specifically.

> > 
> As this is now into imap territory I think David will probably spot the error
> faster than me, so reassigning to him, hope that's ok.

sure, np.
Summary: Select Offline -> Download / Sync Now doesn't go offline → Select Offline -> Download / Sync Now doesn't go offline (not downloading all emails)
Whiteboard: [needs patch]
Attached patch correctness fix (obsolete) — Splinter Review
I'm still working on reproducing this, but looking at the code, we may not be sending the last notification that would take us offline.
Attached patch proposed fixSplinter Review
the key change is in nsImapMailFolder.cpp - we weren't notifying the imap protocol code about bodies to download if there were no bodies to download and we were downloading for offline use. This hung the imap thread so that we never got to the stop running url call. I'll look at augmenting the offline download test to catch this.

I also added pause and resume to the autosyncmanager so that it wouldn't kick in while offline download was going on. I also made the autosync manager pay attention to the offline/online notifications, and pause/resume auto sync appropriately.

And I made a couple correctness changes to nsImapOfflineSync that I noticed by code inspection, to make sure that we always send a final notification when done.
Attachment #374847 - Attachment is obsolete: true
Attachment #375096 - Flags: superreview?(bugzilla)
Attachment #375096 - Flags: review?(bugzilla)
Whiteboard: [needs patch] → [has patch for review]
this patch makes the test fail w/o the main patch.
Attachment #375103 - Flags: review?(bugzilla)
Comment on attachment 375096 [details] [diff] [review]
proposed fix

>+      observerService->RemoveObserver(this, NS_IOSERVICE_OFFLINE_STATUS_TOPIC);
>+      observerService->RemoveObserver(this, NS_IOSERVICE_GOING_OFFLINE_TOPIC);
>+      
>     }

nit: no need for the blank line.
Attachment #375096 - Flags: superreview?(bugzilla)
Attachment #375096 - Flags: superreview+
Attachment #375096 - Flags: review?(bugzilla)
Attachment #375096 - Flags: review+
Attachment #375103 - Flags: review?(bugzilla) → review+
fix checked in, along with augmented test case.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [has patch for review]
You need to log in before you can comment on or make changes to this bug.