With a slow server, polling a news server for new headers can take a significant amount of time. I notice it in 4.x and I find it annoying. A pref would be nice. As I see it, this is one of those areas where user preference is important.

it doesn't poll for new headers, it just does a "GROUP" command for each newsgroup you are subscribed to. but this can easily be made into a pref. changing summary. this would be a good help wanted bug. I'll probably re-assign it to nobody@mozilla.org

making this a help wanted bug.

adding tswan to the cc list. he's looking into fixing this bug.

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

tenthumbs, is this a dup of #103010? I'm not sure if you meant not to do "GROUP" on startup, or on any expand.

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

Comment on attachment 130360 [details] [diff] [review] Patch doesn't apply any more

updated patch

Comment on attachment 151600 [details] [diff] [review] Patch (updated) > if (isServer == "true") > { >- var server = msgFolder.server; >- server.performExpand(msgWindow); >- } >+ if (pref.getBoolPref("news.group_on_expand")) >+ { >+ var server = msgFolder.server; >+ server.performExpand(msgWindow); >+ } >+ } You forgot to check that the server is a news server here. I think that by not calling updateFolder you will will miss out more than simply not getting new messages. It may be better to implement these preferences in the news back end somewhere.

(In reply to comment #10) > I think that by not calling updateFolder you will will miss out more than > simply not getting new messages. It may be better to implement these > preferences in the news back end somewhere. AutoCompact won't be called when not executing UpdateFolders. Where in the backend should I place it? nsMsgNewsFolder::UpdateFolder and nsNntpIncomingServer::PerformExpand?

Moved the code to the backend. Also fixed a warning: c:/Mozilla/mozilla/mailnews/news/src/nsNewsFolder.h: In constructor ` nsMsgNewsFolder::nsMsgNewsFolder()': c:/Mozilla/mozilla/mailnews/news/src/nsNewsFolder.h:153: warning: ` nsMsgNewsFolder::mUnsubscribedNewsgroupLines' will be initialized after c:/Mozilla/mozilla/mailnews/news/src/nsNewsFolder.h:149: warning: ` PRPackedBool nsMsgNewsFolder::m_downloadMessageForOfflineUse' c:/Mozilla/mozilla/mailnews/news/src/nsNewsFolder.cpp:122: warning: when initialized here There is one problem with the patch: The busy cursor doesn't go away. I currently have no Idea what's the way to handle this. Neil?

Neil isn't on the CC list...

Comment on attachment 152604 [details] [diff] [review] Patch v2 >+ // GetNewMessages has to be the last rv set before we get to the next check, so >+ // that we'll have rv set to NS_MSG_ERROR_OFFLINE when offline and send >+ // a folder loaded notification to the front end. >+ rv = GetNewMessages(aWindow, nsnull); >+ } >+ if (rv == NS_MSG_ERROR_OFFLINE) >+ { >+ rv = NS_OK; >+ NotifyFolderEvent(mFolderLoadedAtom); >+ } >+ return rv; >+ } >+ return NS_OK; > } Try changing this to rv = GetNewMessages(aWindow, nsnull); } if (rv != NS_MSG_ERROR_OFFLINE) return rv; } // We're not getting messages because either get_messages_on_select is // false or we're offline. Send an immediate folder loaded notification. NotifyFolderEvent(mFolderLoadedAtom); return NS_OK; } Also a second diff with -w would be nice for review.

Comment on attachment 152681 [details] [diff] [review] Patch v2a (diff -w) >+ // Only if news.get_messages_on_select ist true do we get new messages automatically Nit: you forgot to translate "ist" to "is" :-P > PRPackedBool mGettingNews; > PRPackedBool mInitialized; >- PRPackedBool m_downloadMessageForOfflineUse; >- PRPackedBool m_downloadingMultipleMessages; > > nsCString mOptionLines; > nsCString mUnsubscribedNewsgroupLines; >+ >+ PRPackedBool m_downloadMessageForOfflineUse; >+ PRPackedBool m_downloadingMultipleMessages; >+ I don't think this change is correct...

(In reply to comment #17) > I don't think this change is correct... Why? What must be done instead?

I don't think you should move those variables, they're better where they were.

Comment on attachment 152728 [details] [diff] [review] Patch v2b (diff -w) >Nit: you forgot to translate "ist" to "is" :-P Corrected. >I don't think you should move those variables, they're better where they were. Removed that.

Comment on attachment 152728 [details] [diff] [review] Patch v2b (diff -w) >+ // Only if news.update_unread_on_expand ist true do we update the unread counts "ist" again... don't bother with a new patch until after your superreview.

Comment on attachment 152728 [details] [diff] [review] Patch v2b (diff -w) The spelling mistake Neil pointed out will be corrected before checkin.

Comment on attachment 153023 [details] [diff] [review] Patch v2c (diff -w) Corrected the spelling mistake. Taking r/sr from last version.

I checked in attachment 153022 [details] [diff] [review].

v

Umm, does TB3.1 even automatically download headers (not only GROUP) on expand? Is this another change which makes this one obsolete?