359653 - Consolidate Offline Management in the Front End

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Scott MacGregor]

Currently, we have offline functionality spread out in many JS files in the thunderbird front end. Some of it is still code living in xpfe\communicator\utilityOverlay.js.

We should have a single offline manager class in the front end, consolidating offline functionality to this class. Having all of the pieces in one place will make it easier to implement and more importantly to make adjustments to Bug 359134 (integrate with necko's automatic online / offline manager)

Comment 1

[Scott MacGregor]

Attachment: work in progress

more details coming soon...

Comment 2

[Scott MacGregor]

Attachment: the fix

Comment 3

[Scott MacGregor]

Attachment: New MailOffline object

For review purposes, I think it's easier to see mail-offline.js instead of trying to read the diff for that file. Since the old code in that file has been removed.

Comment 4

[Scott MacGregor]

Overview of the changes

1) We have a new object, MailOffline which is responsible for all things offline related in the mail 3-pane and stand alone message windows. I've moved everything into this class instead of having it spread out between various files and pieces in the front end.

2) We used to depend on a lot of code from http://lxr.mozilla.org/mozilla/source/xpfe/communicator/resources/content/utilityOverlay.js for updating offline status. I've ripped all of that out and implemented it in MailOffline. I had to fork this file from xpfe with the few methods that we still use. I'll file a separate bug to get rid of utilityOverlay.xul/.js all together in mozilla/mail.

3) MailOffline is now the observer for network:offline-status-changed topics instead of commandglue.js

4) There was some stale secure browser UI code that was commented out which I removed.

5) mail-offline.js and mailWindowOverlay.js had duplicate copies of several methods and variables (InitPrompts, gPromptService, gOfflineManager, gOfflinePromptsBundle, etc). With the new implementation all of that went away.

6) We had a lot of places where we used the following pattern:
if(CheckOnline()) 
  GetMessagesForInboxOnServer(server);
else if (DoGetNewMailWhenOffline())
  GetMessagesForInboxOnServer(server);

I fixed those to look more like:

if (MailOffline.isOnline() || MailOffline.getNewMail())
  GetMessagesForInboxOnServer(server);

7) I made some UI changes to the download before going offline and send unsent messages when going online dialogs. They now only have two buttons: Yes or No buttons. No more cancel and no more custom button labels. I also got rid of the line returns in these string entities.

Comment 5

[Scott MacGregor]

Comment on attachment 244840 [details] [diff] [review]
the fix

Enjoy...see comment 7 for a break down of the changes. I also documented mail-offline pretty heavily about the changes there.

:)

Comment 6

[Scott MacGregor]

I filed Bug 359748 to remove our remaining dependencies on communicator's utilityOverlay.

Comment 7

[David :Bienvenu]

Comment on attachment 244840 [details] [diff] [review]
the fix

looks good...MailOffline might be called something like MailOfflineMgr, but overall this looks like a lot of good cleanup

Comment 8

[Scott MacGregor]

Attachment: updated fix with David's review comments

renaming the MailOffline to MailOfflineMgr

Comment 9

[Scott MacGregor]

Comment on attachment 244866 [details] [diff] [review]
updated fix with David's review comments

fixed on the trunk. We'll let it bake there first for a bit.

Comment 10

[Scott MacGregor]

Attachment: small follow up fix

Small follow up fix. We are showing "Sending Unsent Messages" in the status bar when you go back online even if you don't have any unsent messages. We are showing the status string too early in ::SendUnsentMessages, wait until we know we have messages to send.