538242 - window.moveTo(0,0) has no effect

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Jonathan Louie]

Attachment: test case

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7

Will attach test case.  window.moveTo(1,1) works fine.

Comment 1

[Jonathan Louie]

Bug does not occur on Windows (3.5.7) or in Camino (2.0.1).

Comment 2

[philippe (part-time)]

What is your pref setting  for 'allow scripts to move or resize existing windows' under Preferences > Content, the 'Advanced' button next to 'Enable Javascript' ?
Or in About:config: dom.disable_window_move_resize
If it is set to 'true' (unchecked in the Preferencea window), then this bug is invalid.

Comment 3

[Jonathan Louie]

As I said, moveTo's to other locations work fine.  You can see this in my test case.  Yes, "Allow scripts to move or resize existing windows" is checked.

Comment 4

[paul.tabet]

I have the same problem using exactly the following parameters :

A first window is used to open a popup in JS with the following sequence :

window.open('page.php?PHPSESSID=mysessionid&op=do_function', 'Page_Title', 'scrollbars=no,status=no');

This popup has a function to resize itself upon loading :

function ResizeWindow(){
    Width = 800;
    Height = 400;
    window.resizeTo(Width,Height)
    StartX = eval(screen.width/2-Width/2);
    StartY = eval(screen.height/2-Height/2);
    self.moveTo(0,0);
}

There is also a form which the user must validate. Upon validating, we are redirected to a new page inside the same popup. This new page also has a resize feature in order for the window to use all available space onscreen :

function ResizeWindow(){
    self.moveTo(0,0);
    window.resizeTo(screen.availWidth,screen.availHeight);
}
	
The bug presents itself as follows :

The first call to Popup resizing works fine : on my website, the popup opens width a defined size and is centered on screen. But, after validating the form inside the popup, the seconde ResizeWindow function will not work as it should. The ResizeTo call is executed as it should but the moveTo call has no effect whatsoever.

Tried :

Inverting sequence of calls, changing moveTo coordinates, adding other moveTo calls afterwards and also with a timer but nothing works. The moveTo function seems to be completely disabled.

Bug does not appear on PC, only on MAC OS with Firefox 3.6, tested on my MAC laptop and on my clients computers, which all are recent MAC OS X based with at least Firefox 3.6.

Comment 5

[Markus Stange [:mstange]]

Attachment: v1

nsCocoaWindow::Move compares the passed coordinates to mBounds.x/y, but the mBounds position hasn't been updated properly, so it's still (0,0). We need to make sure to update mBounds properly.

Comment 6

[Josh Aas]

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

I talked to Markus about this on irc, rather not update mBounds from a getter, we should do it where the position changes.

Comment 7

[Markus Stange [:mstange]]

Attachment: v2

Comment 8

[Josh Aas]

Comment on attachment 434632 [details] [diff] [review]
v2

> - (void)windowDidMove:(NSNotification *)aNotification
> {
>   // Dispatch the move event to Gecko
>+  mGeckoWindow->UpdateBounds();
>   nsGUIEvent guiEvent(PR_TRUE, NS_MOVE, mGeckoWindow);

Move the update call to before the comment, which doesn't apply to it.

Otherwise looks good, thanks!

Comment 9

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/77e1ae41987e

Comment 10

[Markus Stange [:mstange]]

Backed out: http://hg.mozilla.org/mozilla-central/rev/f85c2efb4eaf
because of test failures:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1269511430.1269512812.30906.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1269511430.1269512755.30736.gz

Comment 11

[Josh Aas]

Attachment: bad sheet size

My Minefield nightly build has incorrectly-sized sheets. Was it possibly generated while the patch here was in the tree?

Comment 12

[Markus Stange [:mstange]]

Yes, it was.

Comment 13

[Markus Stange [:mstange]]

Attachment: v3

going to push this to the tryserver before requesting review

Comment 14

[Markus Stange [:mstange]]

Attachment: v4

Tryserver is all green.

So it turns out that things are working right now pretty much by accident. To make things work correctly with up-to-date mBounds, two more changes were necessary: nsCocoaWindow needed to implement GetClientBounds, and we need to stop calling Show on the hidden window.

Implementing GetClientBounds is necessary to make size-to-contents work correctly. Before this patch that only worked because nsBaseWidget's implementation of GetClientBounds returned an outdated zero-sized mBounds which happened to make things  work most of the time.

Calling Show on the hidden window didn't do anything before this patch since there's a !mBounds.IsEmpty() condition in nsCocoaWindow::Show, and mBounds was always 0x0 for the hidden window. Now that mBounds gets updated properly, it's sized 1x23 at that point (1x1 inner size + title bar), so the hidden window was made visible and suddenly started to accept focus (but it was still offscreen).

I've added a test for moveTo(0,0) but it passes even without this patch.

Comment 15

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/aeeb631c6d43