Last Comment Bug 715867 - sizemodechange event fires during resizing on mac, even when sizemode/windowState doesn't change
: sizemodechange event fires during resizing on mac, even when sizemode/windowS...
Status: VERIFIED FIXED
[inbound]
: testcase
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: x86 Mac OS X
: -- minor (vote)
: mozilla12
Assigned To: Markus Stange [:mstange]
:
Mentors:
Depends on:
Blocks: 429954
  Show dependency treegraph
 
Reported: 2012-01-06 05:37 PST by Nickolay_Ponomarev
Modified: 2012-02-05 12:45 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (open as chrome) (926 bytes, application/vnd.mozilla.xul+xml)
2012-01-06 05:38 PST, Nickolay_Ponomarev
no flags Details
v1 (8.35 KB, patch)
2012-01-27 11:50 PST, Markus Stange [:mstange]
smichaud: review+
Details | Diff | Splinter Review

Description Nickolay_Ponomarev 2012-01-06 05:37:45 PST
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 2012-01-06 05:38:39 PST
Created attachment 586395 [details]
testcase (open as chrome)
Comment 2 Steven Michaud [:smichaud] (Retired) 2012-01-06 12:02:20 PST
Have you tested on other platforms, like Windows or Linux?
Comment 3 Nickolay_Ponomarev 2012-01-06 13:23:16 PST
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 2012-01-06 13:28:44 PST
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) 2012-01-06 13:58:38 PST
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) 2012-01-06 14:02:15 PST
> 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 2012-01-06 14:13:33 PST
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 2012-01-06 14:21:30 PST
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] 2012-01-27 11:50:46 PST
Created attachment 592200 [details] [diff] [review]
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) 2012-01-30 09:51:30 PST
Comment on attachment 592200 [details] [diff] [review]
v1

Looks fine to me.
Comment 12 Ed Morley [:emorley] 2012-01-31 08:50:20 PST
https://hg.mozilla.org/mozilla-central/rev/d2bf6d11d889
Comment 13 Nickolay_Ponomarev 2012-02-05 12:45:58 PST
Thanks, this fixes the problem for me.

Note You need to log in before you can comment on or make changes to this bug.