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

VERIFIED FIXED in mozilla12

Status

()

Core
Widget: Cocoa
--
minor
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: Nickolay_Ponomarev, Assigned: mstange)

Tracking

({testcase})

Trunk
mozilla12
x86
Mac OS X
testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
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'.
(Reporter)

Comment 1

6 years ago
Created attachment 586395 [details]
testcase (open as chrome)
Have you tested on other platforms, like Windows or Linux?
(Reporter)

Comment 3

6 years ago
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.
(Reporter)

Comment 4

6 years ago
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');
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?
> 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?
(Reporter)

Comment 7

6 years ago
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.
(Reporter)

Comment 8

6 years ago
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 )
(Assignee)

Comment 9

5 years ago
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.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #592200 - Flags: review?(smichaud)
Comment on attachment 592200 [details] [diff] [review]
v1

Looks fine to me.
Attachment #592200 - Flags: review?(smichaud) → review+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2bf6d11d889
Whiteboard: [inbound]
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/d2bf6d11d889
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

5 years ago
Thanks, this fixes the problem for me.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.