Closed Bug 538242 Opened 12 years ago Closed 11 years ago

window.moveTo(0,0) has no effect

Categories

(Core :: Widget: Cocoa, defect)

1.9.1 Branch
All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b9

People

(Reporter: louie, Assigned: mstange)

References

Details

Attachments

(3 files, 3 obsolete files)

Attached file 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.
Bug does not occur on Windows (3.5.7) or in Camino (2.0.1).
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.
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.
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.
Attached patch v1 (obsolete) — Splinter Review
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.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #434565 - Flags: review?(joshmoz)
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.
Attachment #434565 - Flags: review?(joshmoz) → review-
Attached patch v2 (obsolete) — Splinter Review
Attachment #434565 - Attachment is obsolete: true
Attachment #434632 - Flags: review?(joshmoz)
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!
Attachment #434632 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/77e1ae41987e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Component: General → Widget: Cocoa
Product: Firefox → Core
QA Contact: general → cocoa
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a4
Version: 3.5 Branch → 1.9.1 Branch
Attached image bad sheet size
My Minefield nightly build has incorrectly-sized sheets. Was it possibly generated while the patch here was in the tree?
Yes, it was.
Depends on: 554906
Attached patch v3 (obsolete) — Splinter Review
going to push this to the tryserver before requesting review
Attachment #434632 - Attachment is obsolete: true
Attached patch v4Splinter Review
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.
Attachment #496811 - Attachment is obsolete: true
Attachment #497247 - Flags: review?(joshmoz)
Attachment #497247 - Flags: review?(joshmoz) → review+
Attachment #497247 - Flags: approval2.0+
http://hg.mozilla.org/mozilla-central/rev/aeeb631c6d43
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.3a4 → mozilla2.0b9
You need to log in before you can comment on or make changes to this bug.