onclose event never fires for Chrome windows with menubar hidden

RESOLVED INVALID

Status

()

Firefox
General
RESOLVED INVALID
5 years ago
3 years ago

People

(Reporter: morac, Unassigned)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
I noticed that the onclose event is not firing when browser windows are closed.  I don't know when this stopped working exactly, but it doesn't work in Firefox 10 all the way through the nightly loads.  It does work in SeaMonkey.

I'm not sure if this was removed or not, but it's still listed on the XUL Events page.  https://developer.mozilla.org/en-US/docs/XUL/Events#Common_XUL_events

My add-on is listening for this and a recent change I made exposed the fact that it was never firing.

Steps to reproduce:

1. Open the error console and copy and paste the following into the Error console:

Components.utils.import("resource://gre/modules/Services.jsm"); var win = Services.wm.getMostRecentWindow("navigator:browser"); win.addEventListener("close", function() { alert("closing") }, false);	

2. Close the most recent browser window using the "X" button.


Expected results:  
A "closing" alert should pop up before the window closes.

Actual results (in Firefox):
No alert ever occurs as the "onclose" event never fires.
(Reporter)

Updated

5 years ago
Component: Tabbed Browser → Event Handling
Product: Firefox → Core

Comment 1

5 years ago
Is it alert() which isn't working or 'close' event?

Updated

5 years ago
Keywords: regressionwindow-wanted
(Reporter)

Comment 2

5 years ago
The "close" event.  I'm not sure when this stopped working, but I went back to Firefox 10 and it didn't work there either.  It does work in SeaMonkey so I'm not sure if this is core or Firefox.

Comment 3

5 years ago
Could you try to find the regression range?
Nightly builds are available here http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/
(Reporter)

Comment 4

5 years ago
I'll try when I get a chance, but as I mentioned it doesn't work in Firefox 10 so it's been broken at least a year. 

If someone knows where the "close" event comes from I can look into that.
(Reporter)

Comment 5

5 years ago
Okay I'm confused a bit.  I originally thought this wasn't working in the latest loads because I pulled data from three different people's PC and it wasn't working for any of them (including myself), but I ended up creating a brand new profile  and it worked with the nightly load. 

That's odd because I had disabled all add-ons in my old profile and it still didn't work.  I then started copying files (no add-ons) over from my old profile to the new one until it broke.  The file it broke on was localstore.rdf.

I'm not sure why anything in localstore.rdf could break the sending of the "close" event for the browser chrome window. I'm in the process of editing it down to the smallest version that still causes the problem.
(Reporter)

Comment 6

5 years ago
Okay I found the trigger.  If the menu bar is hidden, the "close" event won't fire.

Steps to reproduce in a new Firefox profile:
1. Right click on toolbar and click "Menu Bar" so it doesn't display.
2. Open error console and copy and paste script from comment #1.
3. Close browser window.

The "close" event doesn't fire.  Repeating the steps with the Menu Bar displayed and the "close" event does fire.

Since it's a problem with the Menu Bar being hidden, I'm not sure if this is an "Event Handling" issue or a "XUL" issue.
Summary: onclose event never fires for Chrome windows → onclose event never fires for Chrome windows with menubar hidden
(Reporter)

Updated

5 years ago
Keywords: regression, regressionwindow-wanted

Updated

5 years ago
Component: Event Handling → General
Product: Core → Firefox

Comment 7

5 years ago
(In reply to Michael Kraft [:morac] from comment #6)
> Okay I found the trigger.  If the menu bar is hidden, the "close" event
> won't fire.
> 
> Steps to reproduce in a new Firefox profile:
> 1. Right click on toolbar and click "Menu Bar" so it doesn't display.
> 2. Open error console and copy and paste script from comment #1.
> 3. Close browser window.
> 
> The "close" event doesn't fire.  Repeating the steps with the Menu Bar
> displayed and the "close" event does fire.
> 
> Since it's a problem with the Menu Bar being hidden, I'm not sure if this is
> an "Event Handling" issue or a "XUL" issue.

But I keep my menu bar visible all the time when I reported the issue on mozillaZine

Comment 8

4 years ago
Using DOMInspector, I found out that disabling the menubar enables a custom title bar (id="titlebar") with custom buttons (id="titlebar-close"). This is on FF. On TB, the custom title bar is always shown.
The difference is that the titlebar-close button uses oncommand="BrowserTryToCloseWindow()" which uses window.close(), which fires 'DOMWindowClose'. On TB, window.close() is called directly. Whereas clicking on the close button of the native window (widget/window/nsWindow.cpp) fires the 'close' event, obviously through nsWebShellWindow::RequestWindowClose().

As it is expected that the fake/custom close button has the same behavior as the native one, the titlebar-close button should probably also create and dispatch a 'close' event.

Comment 9

4 years ago
Created attachment 8389938 [details] [diff] [review]
basic close event firing

I learned that the titlebar was needed for supporting Moz themes.

Creating and dispatching the 'close' event is a straightforward solution. Of course, tests are missing, and this should be ported to Thunderbird.
But the problem remains with the fake minimize button, which does window.minimize() and doesn't trigger WM_SYSCOMMAND on windows...
Which leads me to think that these buttons should probably call _native_ behaviors: something like window.nativeClose() and window.nativeMinimize(). I don't know which platforms are concerned apart from winnt.
And I'm not sure this approach is relevant, so I'll have to seek feedback here.

Updated

3 years ago
Flags: needinfo?(dao)

Updated

3 years ago
Duplicate of this bug: 746583

Updated

3 years ago
Duplicate of this bug: 873055

Comment 12

3 years ago
As an addon developer, I overcame the problem simply by replacing the command/oncommand attributes of the titlebar buttons, if present.
Hopefully Dao will take a look at this issue. I've been told he's the most qualified for helping here.
This is expected behavior. The close event is supposed to tell front-end code that there's an attempt to close the window from outside of the browser chrome, e.g. native caption buttons. It sounds like you want to use the close event for something it isn't intended for, e.g. to catch all attempts to close the window.
Flags: needinfo?(dao)
(Reporter)

Comment 14

3 years ago
(In reply to Dão Gottwald [:dao] from comment #13)
> This is expected behavior. The close event is supposed to tell front-end
> code that there's an attempt to close the window from outside of the browser
> chrome, e.g. native caption buttons. It sounds like you want to use the
> close event for something it isn't intended for, e.g. to catch all attempts
> to close the window.

I originally filed this bug because originally the close event fired for all window closes.  That was changed at some point to only fire when the menu bar is visible.  I can't see that being the expected behavior.

Comment 15

3 years ago
Mmh, as this report and the related ones show, this is not the expected behavior. Title bar buttons, whatever the implementation underneath, are expected to behave as such. In fact, beside front-end Moz devs, nobody knows that they are copies.
I discussed with dao on IRC, but we couldn't understand each other.
(In reply to Michael Kraft [:morac] from comment #14)
> I originally filed this bug because originally the close event fired for all
> window closes.

No, it wasn't. It was and still is only fired for close messages we get from the OS. This also excludes Shift+Ctrl+W and File > Close Window, for instance.

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.