715867 - sizemodechange event fires during resizing on mac, even when sizemode/windowState doesn't change

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Nickolay_Ponomarev]

STR:
1. Open the attached testcase as chrome (I use Developer Assistant's XUL Editor to test) on Mac.
2. Click the button -- a window will open
3. Open the Error Console
4. Resize the window opened in step 2
--> the error console will show 'sizemodechange' events firing as you resize the window
5. Maximize the window
--> the error console will show 'sizemodechange' events firing as the window's size animates

Expected results: sizemodechange fires only when window.windowState changes.

As far as I can see, the widget change firing NS_SIZEMODE was made in bug 429954 (http://hg.mozilla.org/mozilla-central/rev/71f1dacf8bdd).

The event was exposed to DOM ('sizemodechange') in bug 648045.

I'm filing this to note in the docs for 'sizemodechange'.

Comment 1

[Nickolay_Ponomarev]

Attachment: testcase (open as chrome)

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Have you tested on other platforms, like Windows or Linux?

Comment 3

[Nickolay_Ponomarev]

Huh, I thought I mentioned that...

Comment 0 was for Mac nightly on 10.7. I also tested Firefox 9.0.1 on XP - it fires sizemodechange twice on maximize/minimize, but doesn't fire it continuously during or after resizing, only on window mode changes.

Comment 4

[Nickolay_Ponomarev]

I just realized there is an easier way to reproduce using the Web console: open about:config (or another privileged page) in a Firefox tab, open Web Console, then run the following in the console. This takes care of steps 1 and 2 in comment 0.

var winXUL = '<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>' +
    '<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">' +
    '<script type="text/javascript">' +
    '<![CDATA[' +
    'var aConsoleService = Components.classes["@mozilla.org/consoleservice;1"].' +
    '     getService(Components.interfaces.nsIConsoleService);' +
    'window.addEventListener("sizemodechange", function() {' +
    '  aConsoleService.logStringMessage("sizemodechange: " + window.windowState);' +
    '}, false);' +
    ']]'+'>' +
    '</script>' +
    '</window>';
window.open('data:application/vnd.mozilla.xul+xml,' + winXUL,
                 '', 'chrome, resizable, width=100, height=100');

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Thanks for letting us know that you've tested on Windows, with different results.

I haven't yet tried to reproduce this bug, or to find the relevant code.  But one question occurs to me -- is it possible that window.windowState isn't changing often enough on OS X?

Comment 6

[Steven Michaud [:smichaud] (Retired)]

> is it possible that window.windowState isn't changing often enough on OS X?

I don't actually know the possible values of window.windowState.  If it's just things like "minimized" or "maximized", then my question doesn't make much sense.

But then the question becomes -- why should sizemodechange fire only when window.windowState changes?

Comment 7

[Nickolay_Ponomarev]

Sorry I wasn't clear. Your guess about possible values of windowState is correct, see: http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/base/nsIDOMChromeWindow.idl#46

The change I linked to in comment 0 has this in windowDidResize:
    2.63 +  // Resizing might have changed our zoom state.
    2.64 +  mGeckoWindow->DispatchSizeModeEvent();

that fires NS_SIZEMODE event, in response to which nsWebShellWindow::HandleEvent fires the sizemodechange event. The (arguably minor) issue is that no code between the windowDidResize and nsWebShellWindow::HandleEvent checks that the "size mode" (which is I think supposed to refer to the window state - minimized, maximized, fullscreen, or normal) actually changed.

Comment 8

[Nickolay_Ponomarev]

And to actually answer the question "why should sizemodechange fire only when window.windowState changes?" (sorry it took me 8 comments, should have been in comment 0):

because that's, as far as I can see, is the intent of that event.

We already have 'resize' for resizes; bug 648045 needed to add notification fired when a window is minimized. bz did it by exposing NS_SIZEMODE internal event to the chrome, which on Windows (and probably on Mac before the change for bug 429954) only fires when the window state changes.

(btw, sizemode is also used as a name of a XUL attribute storing the normal/maximized state of the window: https://developer.mozilla.org/en/XUL/window#a-sizemode )

Comment 9

[Markus Stange [:mstange]]

Attachment: v1

This stores the current sizemode in mSizeMode and only fires an event if the new sizemode is different from it. In order for that to work, mSizeMode may only be changed inside DispatchSizeModeEvent, and not in SetSizeMode (as was the case before). Test included.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 592200 [details] [diff] [review]
v1

Looks fine to me.

Comment 11

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2bf6d11d889

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d2bf6d11d889

Comment 13

[Nickolay_Ponomarev]

Thanks, this fixes the problem for me.