Linux 2002051108 and on current CVS... The little plugging wires icon to switch online/offline mode in the lower right corner of MailNews doesn't work anymore. However it does still work from within the Browser.
Caused by checkin of bug 102517.
I see this on W2K 2002051104. Additionally, I can't even change back to online from the menu in the MailNews window. (But as the original reporter mentioned, using a browser window works either with the icon or the menu.) Someone so empowered should change the OS to "ALL".
great. I always like functions who behave contrary do documentation. ok, the problem is: in mailnews/base/resources/content/mail-offline.js, these two functions are called: gOfflineManager.synchronizeForOffline and gOfflineManager.goOnline Now, these functions already change the offline state themselves. So if I return true, line 74 of utilityOverlay.js is reached: ioService.offline = !ioService.offline; Which changes offline state again, and therefor the state is not changed at all.
Ok, on a hunch I had biesi look into this thinking that the native mail functions changed state, then we we returned true, we reversed the online state to back where we started from! So, as partial penalty to him for not testing the patch to 102517, I'm assigning to him. :-)
Assignee: sspitzer → cbiesinger
OS: Linux → All
Created attachment 83261 [details] [diff] [review] patch this patch does two things: in mail-offline.js, calls synchronizeForOffline to be called with goOfflineWhenDone set to false; and it adds an argument to goOnline (goOnlineWhenDone) (and fixes all callers, using true everywhere except in mail-offline.js).
Comment on attachment 83261 [details] [diff] [review] patch r=hwaara Let's get this in ASAP.
Attachment #83261 - Flags: review+
Comment on attachment 83261 [details] [diff] [review] patch the patch looks reasonable to me. Sorry if I sr'ed a bad warnings patch. I'd like to see an r= from the guy who made the original warnings patch, to confirm he knows what he busted :)
Alecf, I already condemned the guy who made the warnings patch to fix this bug in comment #4. He's the one that wrote this patch, too. :-)
*** Bug 144304 has been marked as a duplicate of this bug. ***
16 years ago
Priority: -- → P1
Target Milestone: --- → mozilla1.1alpha
*** Bug 144476 has been marked as a duplicate of this bug. ***
It seems to me like it would be much cleaner and straightforward to have these functions return false so utilityOverlay.js wouldn't diddle the offline state. goOnline SHOULD go online. It also seems like the first patch should be backed out, and the warning fixed correctly, instead of trying to recover from the first patch by changing all this code. Is there a reason we're not trying that instead?
>It seems to me like it would be much cleaner and straightforward to have these >functions return false so utilityOverlay.js wouldn't diddle the offline state. that would be possible too... not sure if that's cleaner; definitely affects less code, though. >goOnline SHOULD go online. ok, if you say that; so in this case it probably is the best to simply return false > It also seems like the first patch should be backed >out, and the warning fixed correctly Well, I tried directly fixing it correctly; since I need r= and sr= anyway, I thought I could fix it directly, instead of backing out and fixing afterwards. I'll attach a patch to return false in a second
> It seems to me like it would be much cleaner and straightforward to have these functions return false It seems odd to have a function always return false. If we go this route, just have it |return;| in all such places and remove the comment which says to return false if we fail. Please add a comment at the start of the function which says that the function always goes online or offline. Still better, do we have any other functions that would be called by the eval in utilityOverlay.js? Should we just let all eval'd functions there handle their own online/offline state, and not even worry about what the eval's return value is? I think that would be the optimal solution...
Created attachment 83597 [details] [diff] [review] patch to change return true->return false ok, can I get r= and sr= for this patch?
Attachment #83261 - Attachment is obsolete: true
>It seems odd to have a function always return false. If we go this route, just >have it |return;| in all such places and remove the comment which says to return >false if we fail. well no... that sounds bad... since the return value is used, we should return false, imho btw, I forgot to remove the comment at the top of the function, I'll do that before checking in
> ok, if you say that; so in this case it probably is the best to simply return false I was agreeing with you that the method should do what its name promises :-) >It seems odd to have a function always return false this function returns false so that toggleOfflineStatus in utilityOverlay.js won't toggle the offline state, because we're taking care of it! We're not the only checkFunc possible - if you look at that code, it might make more sense why we need to return false. It's not a success or failure code; it's an indication to toggleOfflineStatus that it should NOT change the offline state.
thank you, that's much better. I'm sorry I didn't get involved in the earlier warning fix, but I wasn't aware of the semantics of this return value so I didn't know how much mischief could be caused by changing it. What we need is a comment at the beginning of this routine saying that the return value (true or false) controls whether the caller will toggle the offline state. Could you remove the old comment and add a new comment, and then I'll sr that? thx.
Created attachment 83602 [details] [diff] [review] change the comment
Attachment #83597 - Attachment is obsolete: true
Comment on attachment 83602 [details] [diff] [review] change the comment sr=bienvenu
Attachment #83602 - Flags: superreview+
ok, can somebody r= this patch? hwaara or sspitzer maybe?
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
*** Bug 144742 has been marked as a duplicate of this bug. ***
Cannot see this bug anymore with build 2002051504 - WinXP. Thanks !
I work to day with 20020515 (Win95 - talkback/zip) The situation is better, but not satisfactory. If you click dircetly on the 2-plug icon, OK, now it turns off-line. But if you click before elsewhere (newsgroup, or mail), it becomes impossible now to turn off-line (no result when clicking on the icon). At this moment it is necessary to lauch the remote connexion (RTC here), which paradoxically turns now the icon off-line !!!
do you have it configured to automatically download messages for offline use (Edit | Preferences | Offline & Disk Space)? Are you physically connected to the network or not when you click the offline button?
Using commercial trunk on linux 2.2 Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020516 mozilla trunk on mac os 10.1.4 Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.0+) Gecko/20020516 This initially looks fixed. Clicking offline icon in (browser, messenger, composer, addbook, stand alone mail mesg) results in going offline, icon dissconnected. Clicking again results in going online. if you click the offline icon in messenger/seperate mail window, it will result in the download mesg prompt. And when I said download at the prompt, both my mail folders/newsgroups were downloaded.
Using commercial trunk 2002-05-24-08-trunk nt 4.0 2002-05-24-07-trun linux 2.2 2002-05-24-03-trunk mac 9.2.2 2002-05-24-08-trunk mac 10.1.4 Verified clicking icon in any of the windows (browser, messenger,addbook,composer, stand alone mesg) results in icon being disconnected/connected. And clicking the icon in stand alone mail/news mesg or messenger results in the mail download prompt. Also tested going offline via file menus if all windows and the icon matches the state of switching on/offline in file menu marking as verified.
Status: RESOLVED → VERIFIED
*** Bug 148421 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.