Closed Bug 99848 Opened 23 years ago Closed 16 years ago

'Exit' disabled on quicklaunch systray context menu if browser windows are open

Categories

(Core Graveyard :: QuickLaunch (AKA turbo mode), defect)

x86
Windows 98
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX
mozilla1.2alpha

People

(Reporter: jruderman, Unassigned)

References

Details

Steps to reproduce: 1. Launch Mozilla. 2. Enable Quick Launch if it's not already enabled. 3. Right-click the Quick Launch icon in the system tray. Result: 'Exit Mozilla' is disabled. Expected: 'Exit Quick Launch' is enabled. Selecting it exits Quick Launch but not Mozilla.
QA Contact: sairuh → tpreston
-> me Bill, what do you think about changing the current hackish way (which you alluded to) that windows set callback functions -- by setting a random property on the dom window -- to a function on nsIXULWindow, so we can just call this from C++ easily?
Assignee: pchen → blaker
Severity: minor → normal
That might work. We probably still need to be able to get to it from JS also so that the code that handles File->Exit can still work (or, that code needs to change to use the nsXULWindow scheme). So would the nsXULWindow::xxxx implementation just call the js function (if defined)? We definitely need to run this by danm; adding him to cc: list.
Yeah, it would just call the js function. As for calling it from js, we can just call that method on nsIXULWindow if we can get to an nsIXULWindow from js. I asked Ben who asked Dan who said that it was possible, but ugly ...
There's a step missing -- I don't know what hackish attribute is set in the current implementation. Or what you need to do or why, for that matter. But if you gotta add something to nsIXULWindow, then you gotta, one imagines. Try to keep it from getting too ugly now. Dan's secret method for getting to nsIXULWindow from script (hold on...): window.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIWebNavigation).QueryInterface(Components.interfaces.nsIDocShellTreeItem).treeOwner.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIXULWindow); happens to work. In real life of course you'd want to bust it up into separate statements and insert null checks. The part where an nsIWebNavigation happens to actually be an nsIDocShellTreeItem is especially suspect. Also, I notice a window.XULBrowserWindow property set on navigator browser windows. It looks to me like it's not what the name would suggest, but if it is, there ya go. Obligatory disclaimers: it's gross to ask for a XUL Window from a DOM window; inapplicable in embedding situations; don't tell anybody you heard this from me; unsupported (but probably won't break); look out! eek!
That's a good one, Dan. Just further proof of my theory that you can get from any object+interface to any other via some set of QIs, GIs, and getter functions. But I don't think we should do that. Dan, you care about this because I think you'll need this code to fix bug 14807. But maybe we don't need to get to the .xul window from our .js that is currently doing this. That code could continue to call the .js functions directly. That's a little bogus because theoretically one could have a derived nsIXULWindow that didn't implement this new method in that way. But I don't think we'd claim our globalOverlay.js code (or wherever it is) is guaranteed to work with such windows in any case. Another alternative is to put the "exit" logic in some nsAppShellService method. Basically, something like QuitAfterAsking() which would be basically Quit() with the extra call to the window before closing it.
> Another alternative is to put the "exit" logic in some nsAppShellService method. > Basically, something like QuitAfterAsking() which would be basically Quit() > with the extra call to the window before closing it. Yeah, this would also be okay. But in this case are you still suggesting that we implement a method on nsIXULWindow? Or, what would be calling on the window that you refer to?
Yes, we'd still need a new nsIXULWindow method (implemented by calling the javascript property). nsAppShellService::QuitAfterAsking would have to call that. The only difference is that now no JS code would have to get the nsIXULWindow interface.
Yes, but then how does the window specify what js to call? The idea was that nsIXULWindow would have an onLastWindowClosing(nsIQuitObserver foo) method that the window could call in its onload, like it currently does window.tryToClose = foo. But if we don't want to get the nsIXULWindow from js, then I don't see how later (in QuitAfterAsking), we'll know what to call.
This all seems pointless, anyway. Shouldn't returning false from a window's onclose or onunload do the right thing and prevent closing and, therefore, shutdown?
ooo. Like I said, I don't follow the whole line of reasoning in this bug, so I'm probably not completely clear. But comment 8 worries me. "Am I the last window" state seems out of place on a window, especially in the interface. The appshellservice already has knowledge of the open windows and knows about last-window-closed conditions (and has overrides and other complicated malarky that you probably don't want to be second-guessing somewhere else). As Blake mentions in comment 9, it's a known problem that we don't respond to the system's OK-to-close queries quite right. Is that the issue?
The issue is that we'd like to enable the Exit menuitem in the native QuickLaunch menu for its icon, but we can't, because we have no way of asking the window if it's ready for shutdown. This is because the way windows currently set their callback function is by setting a property on the window, tryToClose. IOW, they call window.tryToClose = CanClose(); in their onload, where CanClose() returns a bool saying whether shutdown can continue or not. We can easily call the window.tryToClose function from globalOverlay.js's shutdown code because it's already in js, but accessing that from C++ would be a real pain. (Also, I didn't mean onLastWindowClosing(), I meant onWindowClose() in comment 8. Bad turbo memories.)
re: comment 8 I was envisioning the nxXulWindow method looking for the "tryToClose" property and invoking it if present. Basically, a C++-callable means to do what is done in the JS code that gets invoked when one does File->Exit. That's not as clean as what you suggest, though. re: comment 9 That sounds good. I'm not sure what the semantics of the onclose handler are (onunload is called after the decision has been made, so I don't think it can block closing). I also don't know if there is a way to trigger the onclose handler when closing a window programmitically (other than via "window.close()" in the window itself). E.g., whatever happens downstream from the Close() call at http://lxr.mozilla.org/seamonkey/source/xpfe/appshell/src/nsAppShellService.cpp#349 does not result in the onclose handler getting called, as best I can determine. re: comments 10/11 And note that what we want to do when the user selects "Exit" on the systray icon is pretty much what we want to do when the user does Windows shutdown/logoff/etc. Once the mechanism is in place, then we can use it both places.
So, Dan, what do you think? Is this something that belongs to you, or can you offer insight on how to hook it up so returning false from a window's onclose prevents window closure (and, if applicable, application shutdown)?
Target Milestone: --- → mozilla0.9.9
Keywords: nsbeta1
Component: XP Apps → QuickLaunch (AKA turbo mode)
Blocks: 110882
Blocks: 99940
No longer blocks: 110882
Did this bug morph while I wasn't reading my bugmail? In comment 0, I said "Exit Mozilla" on the Quick Launch menu should be changed to "Exit Quick Launch" and always enabled. It sounds like you're trying to make it "Exit Mozilla" and always enabled.
Perhaps we inadvertantly jumped to a wrong conclusion about how to fix this. Blake: Maybe a simpler fix would be to change the wording and just turn off Quick Launch?
The stuff about window close procedure should I think be fixed anyway, in the long run. Whether we want to tackle it now (I don't think it's much work if Dan can just give me some pointers...) is another question. We could just remove the exit menuitem; I think fixing this would be best.
Nav Triage team: marking nsbeta1+
Keywords: nsbeta1nsbeta1+
Dan, can you answer comment 13 when you get a moment?
So onclose="return false;" already does prevent window closure. Who knew. But returning false from onunload doesn't halt the quitting process. See http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDocumentViewer.cpp#1334 - we don't check for eConsumeNoDefault event status. This is presumably because we don't want to give webpages the ability to prevent shutdown. So it seems like what we want to do is check if it's a xul doc and, if so, check the event status and halt quitting if necessary. But how do we propagate that back to the caller? Is returning failure the right thing to do?
Another alternative, by the way, is putting the tryToClose property on the new nsIDOMChromeWindow. But I still think onunload="return false;" is in line with established protocol.
Target Milestone: mozilla0.9.9 → mozilla1.0
Blocks: 108795
nsbeta1- per ADT triage team. ->1.2
Keywords: nsbeta1+nsbeta1-
Target Milestone: mozilla1.0 → mozilla1.2
Blocks: 132641
Blocks: 75599
No longer blocks: 108795
Blocks: 108795
No longer blocks: 108795
No longer blocks: 75599
Blocks: 75599
(Comments on original message) I think this way works fine. I you want to close al Mozilla Windows: -> go to File->Exit If you want to close quick launch but no other window -> select Disable quick launch from the QL context menu or in the preference window Tested in Mozilla 1.0 RC2
Today in build 2002061908 PC/Win98 I encountered this bug for the first time. However, it is happening regardless of whether I have any browser windows open or not. I tried deleting and reinstalling it and using a new profile, but the problem persists.
I think this bug should be closed, because the reported wanted to close QuickLaunch when Mozilla is running, and this is perfectly posible. To close only QuickLaunch while Mozilla is running: 1. Right click on the QuickLaunch icon on the taskbar 2. Select: Disable QuickLaunch. This will close QuickLaunch WFM in Mozilla 1.0 and Mozilla 1.0.1RC1 running on WinXP
Oliver: "Disable Quick Launch" permanently disables quicklaunch, while the "Exit" item on the quicklaunch menu just closes quicklaunch until the next time you start Mozilla. The bug is that "Exit" is disabled while there are Mozilla windows open.
Comment #25 Hi Jesse, understand. Could it be posible that this behavior is a "feature", not a bug? Anyway, in 1.2.1, Mozilla works just like you explain.
Also seen with nightly build 2003011305 on Windows XP SP1.
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0.2) Gecko/20021216 Win98SE "Exit Mozilla" item is *never* active. Also, mail windows cannot be opened after first mail window is closed. Terminating the Quick Launch tray item permits mail windows to be opened again (only once). (This, of course restarts the Quick Launch component.) Terminated mail windows show up on Window menu as empty, unselectable items.
> Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0.2) The 1.0 branch is basically in maintenance mode -- security and crash fixes only. This is neither...
Isn't this the desired behavior? To exit Mozilla completely, you'd have to close the quicklaunch and all browser windows (or whatever other windows there may be). Thus, you must close everything before you can completely "exit mozilla". I think this is the desired behavior because it keeps users from accidentally closing everything. In any case, this bug has been quiet for a while now, please update on the status of it.
Assignee: bross2 → quicklaunch
QA Contact: tpreston
Quicklaunch/Turbo Mode is no longer supported in Seamonkey 2 and Seamonkey1.X is in the maintenance mode (fixing only security bugs)
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.