Closed Bug 65704 Opened 24 years ago Closed 24 years ago

offline status indicators not changing

Categories

(SeaMonkey :: UI Design, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: alecf, Assigned: sspitzer)

References

Details

Attachments

(15 files)

(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
When you go offline/online by clicking on the offline/online indicator, only the current window's offline indicator changes - all the other windows indicate the old state. I've got an idea for a fix, and it requires a slight addition to necko to broadcast the offline state when it changes. Basically, when the state changes, we use the nsIObserverService to send out a notification that the offline state has changed. Then we implement an observer in each window that updates the respective observer, which in turn updates the UI. I'll implement this all in utilityOverlay.js
Depends on: 44208
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.8
QA Contact: sairuh → tever
ok, I have a fix ready in my tree... I can't attach a patch until the tree opens (long story..)
Whiteboard: fix in hand
*** Bug 44579 has been marked as a duplicate of this bug. ***
Whiteboard: fix in hand
Keywords: mailtrack
Update on my patch, before I finally attach it: I've made the offline status indicator into an XBL widget, the only trick is updating the menuitem in the file menu to switch back and forth between Work Offline and Work Online.
moving key 0.8 bugs that didn't make it to 0.9 (yes, these are important enough to keep, yadda yadda)
Target Milestone: mozilla0.8 → mozilla0.9
Sorry, don't see any traction, bumping off mozilla0.9 train, resetting target milestone
Target Milestone: mozilla0.9 → ---
Target Milestone: --- → mozilla1.0
So this is going to apply for all windows, both browser AND mail windows? Just making sure, just checking... or else we need to get a separate mail bug targeted. It'd be great to have this for beta.
Keywords: nsbeta1
Don't think this is required for Navigator nsbeta1 or rtm. Putterman - does mail need this?
Yeah, I don't think it makes sense to have an offline icon in the windows if it doesn't reflect the correct state. cc'ing jpm in case he wants to take it for his team.
This should be fixed for rtm, as it would be very confusing for users with multiple windows open. Isn't there a patch for this?
Mail needs this - it's a dogfood bug for offline users (never mind catfood). It needs to be fixed for beta. I'm talking to Seth and Alec about solving this somehow.
right, they should all be changing simultaneously, like they did in 4.x but N6 went out in the current condition, I think we can survive beta without it.. definite for rtm though
Target Milestone: mozilla1.0 → mozilla0.9.2
*** Bug 79978 has been marked as a duplicate of this bug. ***
Seth's going to help with this. Thanks, Seth!
I've got the fix for multiple browser windows, just need to fix some minor issues with some mailnews windows. the fix is to utilityOverlay.js when we get cycles, we can redo this as an XBL widget. when I get this fixed, I'll log a new bug on the XBL widget.
Assignee: alecf → sspitzer
Status: ASSIGNED → NEW
looks good to me, r or sr = bienvenu
before I check in, I'm going to polish up that fix a little more and try to get all the offline indicators playing nice. (search, compose, mail.) I hope to have something today.
Status: NEW → ASSIGNED
(editor, history, addrbook, too.)
so far so good. mail and browser UI (indicator, tooltips, and menu item) do the right thing. I'm fixing the mail .js so we observe the offline state and do the appropriate things when the online state changes. this makes MsgToggleWorkOffline() in commandglue.js much simpler and easier to read since the UI elements are already handled by utilityOverlay.js patch coming soon. (bienvenu is going to like it, I tell you what.)
bienvenu: now, if mail is up, and we go offline (or online) from any window, the proper UI will pop up. rough draft of this patch coming soon...
actually, I was hoping that clicking on the offline ui icon wouldn't bring up any ui - only the menu would do that, the way 4.x worked. I wanted clicking the icon to be the minimalistic way of going online/offline.
working on: 1) removing the offline observer when a browser window goes away 2) making sure when new windows open, they get the correct offline state 3) fixing compose, addressbook, etc..
> actually, I was hoping that clicking on the offline ui icon wouldn't bring up > any ui - only the menu would do that, the way 4.x worked. > I wanted clicking the icon to be the minimalistic way of going online/offline. ok, I'll fix it.
your 2nd draft looks like the wrong patch file, but the 1st draft is looking good.. but please make sure the code wraps at 80 columns :)
I'm need to bite the bullet and finish alecf's XBL for the offline indicator. there doesn't seem to be a way to do onload and onunload for the overlay.
maybe not, I think I can do this: addEventListener("load",foo(),false); addEventListener("unload",bar(),false);
> but please make sure the code wraps at 80 columns :) oops, sorry. I'll fix that.
patch coming. please review. I'll continue to work on the mailnews part of this in another bug.
r=bienvenu, thanks, Seth!
slight revision, we don't need to call setOfflineStatus() anymore in navigator.js and mailWindow.js, as the load hander for the utility overlay will set the correct state. new patch on the way
Attached patch new patch (deleted) — Splinter Review
looks good! my only nit is that the IOService now has a contract id (maybe it didn't when this was first written: "@mozilla.org/network/io-service;1" (from nsNetCID.h)
oh, and sr=alecf with that switch. Thanks for taking care of this seth
thanks for the info about the progid. there are few other bugs in bugzilla about switching to it, once it was added. I'll make the fix and then land. then, I'll go finish up all the mailnews offline issues that I started. (and make sure offline indicator is doing the right thing in the rest of the tree.)
anyone know why history has an offline indicator?
I hate to say this, but there are a LOT of people bringing in utilityOverlay.xul - could you fix the onload handler to make sure there is an offline widget, and bail early if there isn't one? should just be a one line addition.. http://lxr.mozilla.org/seamonkey/search?string=utilityOverlay.xul
good catch. I'll work on that now.
I like it. sr=alecf
new version of the patch coming. comments (and remaining issues) to come next.
Attached patch new patch (deleted) — Splinter Review
the last patch gets browser, compose, 3 pane, stand alone msg window all doing the right thing. when the offline state changes, all those windows will update. when going offline in 3 pane or std alone msg window, the user gets prompted before going offline and can cancel the offline state chagne. comments / issues 1) utitlityOverlay is getting loaded multiple times. four times for the 3 pane, and twice for the std alone msg window. that causes me to add the same observer multiple times. when the window goes away, we only remove the observer once. so if the offline state changes after the window goes away, we assert. I added a call to remove the current observer, before adding it. that prevents the assertion because before we add the observer, we remove the previous one. I could have written it so I enumerated over the observers, and didn't add again if a duplicate was found. but this was simpler. of course, I need to figure out why this is happening to those two mail windows. [this multiple overlay problem might indication of a performance problem?] 2) mail has a special requirement. when the user tries to change their offline state, we want to jump in and ask them if they want to do some work before changing state. they can also cancel (and decide not to change their offline state.) I extended utitlityOverlay.js to look for the "offline-status" element, and get the "checkfunc" attribute. if that succeeded, we eval() that func and if that returns false, we bail on the offline state change. otherwise we proceed. right now, only the 3 pane and stand alone message window have this behavior. if you change state in other windows, you will not run the mail "pre-offline" state change func. any suggestions on another way to do this? 3) I was able to remove the onfocus hander for msg compose, and clean up the offline handling. but, to get the utitlityOverlay to work I needed to have something observe the "Communicator:WorkMode" broadcaster. in nav, 3 pane, and std alone msg window, we have a "File | Go Offline..." menuitem. msg compose doesn't have this, so I added it, but made it hidden by default. I need to figure out a no hacky way to get the overlay to overlay without needing a menuitem like that. I think the statusbar overlay would be enough, but I'm too tired to figure that out right now. 4) history, search and subscribe offline indicators are not working properly yet. for similar reasons to #3. if there are more windows that have offline indicators (status bars), we'll need to fix them 5) we're not using it, but I've left in the observer for the mail windows.. If there is any UI that needs to be disabled when offline, that's the place to do this. if none of the mail window UI needs to change, we can remove the observer code. 6) when the msg compose window comes up (in offline mode) the send button doesn't say "Send Later". It will change to that if you change state after the window is open. all that said, I think it's worth landing this before addressing all the issues. we are better off than before, and this will make it better in the common case. [and the offline UI owner can address the other issues]
whoops, I need to fix something minor with msg compose, new patch coming soon.
moving to 0.9.1
Target Milestone: mozilla0.9.2 → mozilla0.9.1
Attached patch patch (deleted) — Splinter Review
new patch coming that fixes #6: > 6) when the msg compose window comes up (in offline mode) the send button > doesn't say "Send Later". It will change to that if you change state after > the window is open.
I agree that it's worth landing this - it will be much better than what we have now.
new patch on the way. it will be up to date with the trunk.
*** Bug 79797 has been marked as a duplicate of this bug. ***
darn, recent changes on trunk will require more updates. new patch coming...
fix landed. now to go log bugs on the open issues. this should help users (and qa) with offline.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Wonderful team work. Please take a look at bug 79952 , which has operational characteristics similar to the one you have solved.
QA Contact: tever → sairuh
QA Contact: sairuh → gchan
looks like one more bug got assigned to me.. Looks very similar to bug 78920 Not sure which build is the official 'beta build'. I tested with this build: 2001-06-07-13-0.9.1/ - win nt 4.0 2001-06-07-03-0.9.1/ - mac os 9.0.4 2001-06-07-13-0.9.1/ -linux 2.2, red hat 7.0 Verifying that clicking the offline indicator in one of these windows results in the other windows displaying the same state: a.Browser b.Composer/Editor page c.Mail/news compose window d.Messenger window e.Separate message window f.Separate newsgroup window g.search messages window h.address book window i.history window j.subscribe to newsgroups window It works as expected in all windows except for history, search messaages, and subscribe to newsgroup window (as the indicators don't work at all which is a known bug 80970) No need to test trunk builds as this fix landed before we began branching. Marking as Verified.
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: