218214 - New windows positioning partly offscreen

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[C.S. Mo]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1

In Camino, if the active window is positioned so that the bottom edge or right
hand edge is either at the edge of the screen, or off-screen, when you make a
new window it will appear in the upper left hand corner of the screen.

Firebird, on the other hand, attempts to position new windows in relation to the
active window even if there is not enough room on the screen. This means that
new windows will appear in semi-random places depending on where space is. I
think that Camino's behavior is much nicer..

Also, Camino allows you to open links from a window that is in the background.
This is nice because it allows you to stack new windows without changing window
layers all the time. (There's already a bug for this in the general Mozilla
section but I'm not sure if it effects Firebird).

Reproducible: Always

Steps to Reproduce:
1. Create a window.
2. Position the window so that either (or both) the right hand border or the
bottom border is against the edge of the visible screen or offscreen.
3. Open a link in a new window, or simply create a new window.

Actual Results:  
The new window is positioned poorly.

Expected Results:  
The new window should be stacked in the upper left hand corner.

I believe that this bug effects all versions of Firebird, however I have not
tested it on a PC or Linux in several versions.

Comment 1

[Jonathan Louie]

confirmed, 0.7 candidate 20030926.  this behavior only appears when the window
is less than full height.

Comment 2

[riscky]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a2) Gecko/20040525
Firefox/0.8.0+

Firefox no longer places new windows partly offscreen, it however doesn't make
its new window (after reaching the bottom left) at the top right. Instead it
makes a new window of the last window (bottom left)

Comment 3

[Asaf Romano (gone)]

I still notice this bug, sometimes.

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20040903
Firefox/1.0 PR (NOT FINAL

Comment 4

[riscky]

I would also agree with Asaf I am also seeing this again.

I tend to think bug 211551 depends on this... very similar bugs (as to what gets
displayed off screen.

Also... dock placment effect this bug... Firefox always seems to honnor the
right edge of the screen but only the bottom if the dock is placed on the bottom.

Comment 5

[Asaf Romano (gone)]

*** Bug 256637 has been marked as a duplicate of this bug. ***

Comment 6

[Asaf Romano (gone)]

*** Bug 262528 has been marked as a duplicate of this bug. ***

Comment 7

[Asaf Romano (gone)]

*** Bug 269448 has been marked as a duplicate of this bug. ***

Comment 8

[L. H.]

*** Bug 271875 has been marked as a duplicate of this bug. ***

Comment 9

[Hiro]

*** Bug 294103 has been marked as a duplicate of this bug. ***

Comment 10

[Asa Dotzler [:asa]]

->josh cuz he really wants this bad and he has half the patch.

Comment 11

[Josh Aas]

*** Bug 237110 has been marked as a duplicate of this bug. ***

Comment 12

[Josh Aas]

*** Bug 220671 has been marked as a duplicate of this bug. ***

Comment 13

[Josh Aas]

*** Bug 281128 has been marked as a duplicate of this bug. ***

Comment 14

[Josh Aas]

Attachment: getscreenbounds patch, for reference only

this does not fix this bug, its just for reference

Comment 15

[Crutcher Dunnavant]

On a mac, with firefox 1.0.4

Full screen windows have the following behaviour:

if you set up a full screen window (stretch it all the way, position it, then
hit the maximize button) on the mac, then create a series of NEW windows.

if The Dock is on the bottom of the screen, AND visible (not auto-hidding), then
the new windows will be full-screen as well.

If the dock is not on the bottom of the screen OR it is auto-hidding, then
firefox will create a window who's status bar is off-screen, and who's top is
shifted down equivalently.

Comment 16

[Crutcher Dunnavant]

Looking at the getscreenbounds path, I see that while adjustment is made for the
dock, no test is made to see if the dock is:

A) Auto-hidding, or
B) On the bottom of the screen.

Comment 17

[Gary Elshaw]

Two differences with this bug: Integration with the dock at bottom, and 'hidden
when not in use'.

1. If the browser window is at full screen the new windows (i tested 5) appear
almost arbitrarily: slightly to the right of the originating window with the
title bar immediately below its predecessor, followed by another that almost
totally obscures the preceding one. The status bar partially disappears after
the first new window and has totally disappeared for the second.

2. However, if the browser window is given a dock-sized margin at the bottom,
each new window will uniformly appear tiled below the preceding window and allow
a greater number of fully viewable windows onscreen.

OS X 10.4.2 Deer Park Alpha 2

Comment 18

[Uri Bernstein]

*** Bug 303659 has been marked as a duplicate of this bug. ***

Comment 19

[Amir Caspi]

Oops, looks like I didn't search on enough keywords when submitting Bug 303659.
 Sorry guys.

However, the description in that bug is different than given here... different
manifestation of the same problem, namely that window placement isn't cycling
once the windows reach a terminal edge (bottom or right).  I now see that it's a
fairly long-standing bug...

Comment 20

[Mark Eichin]

(In reply to comment #17)

I actually see a different manifestation (Mozilla/5.0 (Macintosh; U; PPC Mac OS
X Mach-O; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6; Mac OS X 10.4.2/8C46
tiger)

* full screen firefox window
* dock on the right, always visible
* create new window, it appears one mac-title-bar-height down, same width.
* create another one, it appears to be one-pixel down from the first
* a third one, also appears to be one-pixel down

Successive windows appear at the same offset, they don't "creep offscreen" any
further.  The status line is definitely obscured (offscreen) in all cases,
moving the window back up confirms that it is there.

(Further detail: this is on an (old) 12" Powerbook, with a 1024x768 builtin LCD
screen - that's why full screen windows matter so much :-)

Comment 21

[Jason Sonnenschein]

I can confirm that I find the same problem behavior on Deer Park Alpha 1, OS X 10.4.2.  Also on Firefox 
1.0.4 on OS X 10.3.

One interesting thing... the windows only go so low offscreen, when you keep creating windows; it then 
creates new windows at that same low-level.  It's as if firefox is just off by 50-100 pixels as to where it 
thinks the bottom of the screen is, but is trying to behave well (and won't make windows lower and 
lower, forever.)

I patched Deer Park Alpha 2 with the file attached to this ticket.  It seemed to make the problem worse, 
not better.  

Other thoughts:  Can the code from Camino replace the faulty code in Firefox?  Does an extension have 
access to enough information to correct for this--reposition a window back onto the screen, just after, 
or even before it is drawn

How does a bug get advanced in importance, to get bumped up in the priority list?  This one isn't even 
marked as "confirmed" yet.

Comment 22

[Jason Sonnenschein]

I did some debugging by adding code to browser.js, within firefox...

If you have a window with its bottom at the bottom of the screen, 

window.outerHeight + window.screenY = screen.availHeight

right?

It looks like something is off by about 20 or so pixels, so the equation doesn't work.  How high is the 
Mac menubar?  Is that the 20-25 mystery pixels?  Is the "right thing" for a patch to just subtract 20 
pixels from the screenbounds calculations?  Is there a way to ask Carbon/Cocoa the height of the 
menubar?  Is the right thing to subtract that?  Any Carbon/Cocoa programmers out there?

Comment 23

[Jason Sonnenschein]

Attachment: Workaround firefox extension

Wrote this extension as a workaround, there is a fix.  No warranties, etc.  Use
at your own discretion.

Comment 24

[Jason Sonnenschein]

Attachment: Workaround firefox extension

Wrote this extension as a workaround, until there is a fix.  No warranties,
etc.  Use at your own discretion.

Comment 25

[Josh Aas]

This is a well-understood problem and a patch exists. It'll be in within a week
or two probably.

Comment 26

[Hiro]

*** Bug 308595 has been marked as a duplicate of this bug. ***

Comment 27

[Hiro]

The resize box is not hidden under Doc by fix of bug304089. 
But the position where the window is displayed is not the fit upper left. 

Mac OS X 10.3.9
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051001
Firefox/1.6a1

Comment 28

[Marc Rost]

In September I thought there was light at the end of the tunnel.  What's happening?
I do notice a difference between version 1.0.x and 1.5.x.  When I use the bookmarklet 'resize window to full screen', in version 1.0.7 it would cover the full screen and the status bar is already pushed off the bottom of the page, and a new window would open exactly on top of the parent window.  In version 1.5.0.1 it would resize to cover the full screen (status bar visible), and a new window would open shifted down by some 20 pixels.

Comment 29

[Fred]

I'm using Firefox 1.5 on Mac OS 10.3.9.

When I open a second window over a first full-screen window, the top of this second window cascades just below the first, which is OK. However, its bottom can't be seen below the bottom of my 19" display screen ... can't see the status bar, etc.. This is incorrect; it should resize its bottom so that it doesn't go past the bottom of the screen. Otherwise, I'm forced to grab the whole window, move it up, grab its bottom, move that up, then move the window down again so that its top cascades under the first window ... lots of jockeying back and forth, very tedious.

Opening a third (or fourth, etc.) window has the same problem as the second, and they sit right on top of the second (rather than continuing to cascade their window tops).

Comment 30

[Robert Sayre]

We should pick this up on trunk soon, at the very least, if we still have this problem. 

Comment 31

[Amir Caspi]

This bug still exists in Firefox 2.0 on MacOS X (10.3.9 in my case).  If the current window is positioned to the very right edge of the screen, the next window SHOULD open on the left edge of the screen, but does not - it instead opens on the right edge, under the current window.  This is the same behavior as previously.

Since the last report on this was 2.5 months ago, I figured I'd let everyone know this bug persists and still needs squashing.

Comment 32

[Amir Caspi]

By the way, the extension provided by Jason Sonnenschein does not fix the "left-right cycle" issue, namely that when the current window is at (or beyond) the right edge of the screen, the next new window should appear at the left edge of the screen.  Camino and Netscape respect that behavior, while Firefox (even 2.0) does not - it opens the new window still on the right edge of the screen rather than cycling back to the left.

Believe it or not, this bug is actually the main issue keeping me from switching to Firefox.  It would be great to see this fixed so that the behavior of "new window" is similar to Camino and Netscape, namely that new windows cycle back to the left edge of the screen if the current window is at or beyond the right edge.

Comment 33

[Amir Caspi]

The left-right-cycling portion of this bug is still present in Firefox 2.0.0.2 on MacOS X.  As mentioned in comment #32, Jason Sonnenschein's patch does not address the position cycling issue.

Comment 34

[Jason Sonnenschein]

Although the vertical placement is now better, it's not 100% fixed either.  As of Firefox 2.0.0.2 on my Mac, new windows no longer are placed partially offscreen, but they are placed partially behind the dock.  

FYI, the extension wasn't meant to be a fix, just a workaround (and a hack at that.)  A similar one could fix the left/right problem, but again... it's  a hack that moved the window after it was rendered.  Not an ideal solution.

Is anyone working on this bug (both vertical and horizontal?)  Can we at least get this bug to block Firefox 3.0?  It's honestly my biggest annoyance with Firefox.

Comment 35

[Amir Caspi]

Jason: Agreed, this is certainly my biggest annoyance as well, and is what is keeping me using Netscape 7.x (believe it or not), despite its utter slowness and incredible instability.

I'd love to see this one fixed so I can make Firefox my default browser and scrap Netscape.

Comment 36

[Jason Sonnenschein]

I have a guess that I'm hoping someone can look into furhter.  On my machine, firefox tiles windows behind the dock at the bottom of the screen.  Once the newest tiled window is behind the dock __exactly the height of the menubar__, future windows no longer tile further down.

So the guess is this:  The code calls the carbon function GetAvailableWindowPositioningBounds to assess the screen size without the dock and menubar.  It's called from the GetAvailRect in widget/src/mac/nsScreenMac.cpp.  "Top" is subtracted before it is returned from the GetAvailRect function.  Is it possible that the programmers have misunderstood this function, and that's unnecessary?  Is it possible that the program returns "top" and "height," rather than "top" and "botom"?

As for the left/right stuff.  My guess would be that it's a different problem...  that the code just says if I've reached a screen edge, don't tile any further.  This is just a guess, though... it happens in the vertical too... and I haven't been able to find the code that decides where the next tiled window should go...  I've just found the code that gets screensize.  Does anyone know if Windows/Linux show the same behavior (stop tiling when hitting a screen edge?)

Comment 37

[Jason Sonnenschein]

I've been compiling downloaded source, and out-of-box, firefox2 used carbon code, and firefox3 used cocoa.  Again guessing, but it looks like this code is all getting rewritten for cocoa.

Comment 38

[Amir Caspi]

(In reply to comment #36)
> Does anyone know if Windows/Linux show the same behavior (stop tiling
> when hitting a screen edge?)

On Linux, the behavior is "as expected," i.e. window position cycles all the way left once the right edge is hit.  On Windows, it's a bit weirder - tiling will reverse instead of reset, i.e. once the right edge is hit, new windows are offset to the left (tiled) rather than reset to the left edge and offset to the right.  (I used v2.0.0.1 on Linux, v1.5.0.10 on Windows.)

So behavior on all three platforms is rather inconsistent, but IMHO the behavior under Linux is the "correct" behavior and should be ported to Mac.

Comment 39

[Mike Connor [:mconnor]]

This is more of a core Mac platform bug than a Firefox front end, moving components

Comment 40

[Mike Connor [:mconnor]]

restoring nom

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I cannot fully reproduce it with current trunk and 1.8 branch cvs builds. New windows aren't created partly off-screen but if the dock is at the bottom of the screen and multiple new windows are created the bottom of new windows are placed behind the dock. The left-right issue i cannot reproduce. Moving a window mostly off-screen and creating a new window shows the newly one within the screen. If the former window was shifted right off the screen the new one is placed with the right border at the right side of the screen.

Should bug 130872 be duped to this one?

Comment 44

[Jason Sonnenschein]

I started looking at the source code again, and found a little more information.

1.  I can fix the "windows placed behind the dock" behavior like this:
add to nsScreenCocoa.h:
#import <Carbon/Carbon.h>

In the nsScreenCocoa::GetAvailRect subroutine of the file widgets/src/nsScreenCocoa.mm...
in the subroutine nsScreenCocoa::GetAvailRect
change the line
outHeight = r.height;
to
outHeight = r.height - ::GetMBarHeight();

Or... if you don't want to add the carbon import, I think this would effectively do the same thing:
outHeight = r.height - r.y;

Firefox Devs: Is this the right place to make this fix?
r comes from 

nsRect r = cocoaRectToGeckoRect([mScreen visibleFrame]);

If I read this right, "mScreen visibleFrame" comes directly from the OS, so it should be right.  So the problem probably actually lies in cocoaRectToGeckoRect or in the cascading code that uses this subroutine.  I still can't find the cascading code, though.

2.  Would it be a better option for firefox ask the OS to handle cascading?  The "Window Cascading" section in this document seems to imply that cocoa has routines to do cascading for you:
http://developer.apple.com/documentation/Cocoa/Conceptual/WinPanel/Tasks/SizingPlacingWindows.html

3.  I _can_ reproduce the side-to side cascading problems.  Here's how I'd describe them:

If you cascade firefox enough in OS-X, it will hit the right screen edge.  If you keep cascading until it hits the left side of the screen, cascading stops.

Could a firefox dev please look at this again?  Also, I see that this has been set as non 1.9 gecko blocking.  Please reconsider making this a blocking bug.

Comment 45

[Jason Sonnenschein]

The problem does appear to be in the cascading code, rather than the screen GetAvail code alteration I mention above.  If I move the dock to the left side of the screen (with the above code changes installed) the windows no longer cascade all the way to the screen bottom.  

Would one of the devs be willing to post here which source file holds the os-x window cascading code?

Comment 46

[Jason Sonnenschein]

Dug through things more and found the "window staggering" code.  It doesn't appear to have an option to bypass the stagger code for os-specific cascading routines.  Assuming GTK and Windows have built in cascading code, this might be a good option for the future... let the os handle this task.

The solution described in comment 44 _does_ appear to be the correct solution after all.  The gecko<->cocoa rect code looks fine.  When I move the dock to the side of the screen, Safari doesn't cascade all the way to the screen bottom, either.  It stops tiling approximately where firefox does with the patch installed.  I'm attaching a patch to the ticket.

Comment 47

[Jason Sonnenschein]

Attachment: Proposed fix against cvs 2007-05-26

Adjusts height returned by GetAvailRect.  Fixes vertical tiling problems mentioned in ticket.

Comment 48

[(not currently active) Ted Mielczarek]

Comment on attachment 266470 [details] [diff] [review]
Proposed fix against cvs 2007-05-26

Just FYI, when you put up a patch, you set review? and put in a person's email address to ask for review from.  If they approve of your patch, they'll set review+

I've set the flag to ask for review from Josh, who is our Mac dev lead.

Comment 49

[Jason Sonnenschein]

thanks.  I thought the review tag was to request a review

Comment 50

[Fred]

I'll apologize right off the bat because, not being a developer, I can't follow everything in this discussion. So if I say something stupid and/or redundant please forgive me.

I'm not sure the problem I first outlined is being addressed, because most folks here seem to be talking about the left/right cascading issue. My main issue is that the bottom of subsequently opened windows fall off the bottom of my display screen, and I can't see the status bar which shows link URLs, etc.

Upon launching Firefox, I always expand the main window to full-screen size by dragging its lower right corner (for some reason, clicking the green button only toggles between two differently sized windows both of which are less than full-screen ... in many other Mac apps that button will expand the window to full-screen). And then subsequent windows open as full screen as well, cascading vertically just below the first window's title bar, but not cascading to the right. Each additional window opens exactly over the second window, that is, they don't continue to cascade down below the previous window's title bar.

Today I just noticed that if I don't expand the main window to full screen upon launch, subsequent windows do cascade down *and* to the right.

In any case, speaking strictly for myself, I'm not as concerned about the left/right cascading as I am about needing to see the bottom status bar of subsequent windows ... that's my biggest complaint about Firefox. 

Comment 51

[Colin Barrett [:cbarrett]]

Drive by:

I don't think this is the right place to do this, since we also have windows which do not have titlebars (popup windows). Accounting for the height of the titlebar sounds like a good fix, but I don't think this is the right place.

Seems to me like you'd want to do it in nsCocoaWindow.mm and use the value returned by the titleBarHeightforWindow function instead.

Comment 52

[Jason Sonnenschein]

Colin, are you talking about alterning nsCocoaWindow::StandardCreate ?

The titlebar is already accounted for in that subroutine:

// compensate for difference between frame and content area height (e.g. title bar)
    NSRect newWindowFrame = [NSWindow frameRectForContentRect:rect styleMask:features];

    rect.origin.y -= (newWindowFrame.size.height - rect.size.height);

    if (mWindowType != eWindowType_popup) {
      rect.origin.y -= ::GetMBarHeight();
    }

Comment 53

[Colin Barrett [:cbarrett]]

I know, but it seems like nsScreenCocoa::GetAvailRect could be called on things that don't have titlebars, in which case subtracting r.y would be incorrect. I'm thinking of popup windows (context menus, autocomplete boxes) primarily.

This is all a bit confusing...

Comment 54

[Jason Sonnenschein]

Agreed, it's confusing... but if we're already subtracting for titlebar height, then isn't it unlikely that that's the issue?  Are you suggesting subtracting 2 * menubar height?

And are you sure that the popup windows aren't being affected?  I think the menubar is what.... 20 pixels high, so if a dialog is trying to center itself onscreen, then it's off by half that... 10 pixels... hard to notice.

Again...  I think a "best case" would be to just bypass the window staggering code in gecko/firefox, and use the OS X native routine, but if other OS's all use the firefox stagger code, then I would guess an OS X bypass wouldn't get checked back into the trunk.

The only odd thing I found was in HighestPointOnAnyScreen code...  it grabs the full screen frame to make its calculations.  The full frame should have height of the full screen and have "origin.y" of 0.  HighestPointOnAnyScreen adds these together... unnecessary but shouldn't do any harm.  When I took out the + origin.y the window placement was the same.  

Honestly, though... I'm not really attached to my code.  I'd really just like the bug fixed.  So, whatever Josh will check back into the trunk is fine with me... but I hunted for a while, and honestly I think this is the place to make the change.  Colin, would you like to submit a second patch and just have Josh decide what seems like the appropriate fix?

Comment 55

[Jason Sonnenschein]

the 20 pixels is an underestimate... but still you get my point.... not much screen real estate.

Comment 56

[Jason Sonnenschein]

Ahhh... remembered the reasoning for going after the GetAvailRect.  Unless you're talking about a different routine... I think that the window create functions get passed a rect AFTER the stagger decisions have been made. (Is that right?  If not, where does aRect come from?)  So...  we can't just subtract another menubar height at this point...  if the stagger code had received a different AvailRect, it would have placed the window in a different place (in other words, we don't always want to subtract the menubar height... so we'd have to re-implement the stagger code to decide how to adjust x and y)

I see your point about pop-ups.  I could see the difference if I fill up a bookmark menu with tons of stuff.  It stops say 25 pixels short of where it needs to.

So why is it that the new XUL window placement code seems to be using GetAvail output correctly for popup height/cascade up or down, but not for placing toplevel windows?

Comment 57

[Jason Sonnenschein]

Colin,

I think the following would have OS X take over staggering of toplevel windows after the first.  It would be added in nsCocoaWindow::StandardCreate (line 336 of the file.)   nextWindowLocation needs to be a global NSRect, intialized to size 0x0 origin 0x0.  I don't know how to code that.

else if (mWindowType == eWindowType_toplevel) {
    // cascadeTopLeft does nothing when nextWindowLocation @ zero point
    nextWindowLocation = [mWindow cascadeTopLeftFromPoint: nextWindowLocation];
}

Comment 58

[Jason Sonnenschein]

Attachment: Second proposed fix against cvs 2007-05-26

This uses cocoa's internal window cascade code instead of gecko's, but without altering the gecko machine-independent window positioning code.  Rather, all changes are contained in nsCocoaWindow.mm

Comment 59

[Colin Barrett [:cbarrett]]

Comment on attachment 267030 [details] [diff] [review]
Second proposed fix against cvs 2007-05-26

Drive by review (since Josh asked me to take a look last time)

>+    // if it's a toplevel window, then use cocoa native "cascading" to decide
>+    //     window position, rather than gecko "staggering"

Is Gecko's staggering actually disabled? Is it possible for them to interact, possibly with undesired results?

>+    if (mWindowType == eWindowType_toplevel) {
>+
>+        NSRect cascadedRect;
>+
>+        // Initialize the routine with gecko's choice (loaded from user's prefs)
>+        //     nextWindow will never be 0,0 except at start (due to menubar)
>+         if ((nextWindowPosition.x == 0) && (nextWindowPosition.y == 0)) {
>+             nextWindowPosition.x = *aX;
>+             // gecko origin is top-left, cocoa bottom-left
>+             nextWindowPosition.y = HighestPointOnAnyScreen() - *aY;
>+         } 
>+
>+         /*NSLog(@"Requested window position: %f, %f\n",
>+               nextWindowPosition.x, HighestPointOnAnyScreen() - nextWindowPosition.y ); */

Change this to a printf instead.

>+         // cascadeTopLeft moves the window to the coordinates requested
>+         //     then adjusts position as necessary for screen boundaries
>+         //     it returns suggested position of window for its next run
>+         nextWindowPosition = [mWindow cascadeTopLeftFromPoint: nextWindowPosition];
>+         cascadedRect = [mWindow frame];

Remove the space between the colon and the argument, I'd say.

>+
>+         // return the computed values to gecko
>+         *aX = int([mWindow frame].origin.x);
>+         // gecko origin is top-left, cocoa bottom-left
>+         *aY = HighestPointOnAnyScreen() - int(cascadedRect.origin.y + cascadedRect.size.height);

Maybe it'd be better to use the cocoaRectToGeckoRect function, instead of repeating the logic over and over.

Also, I'm not sure about that cast, I think the prevailing style is a bit different.

>+
>+         /* NSLog(@"Placed at: %d, %d\n", *aX, *aY); */

Change this too.

This is a better patch, but I'm curious to know if the gecko staggering will still happen. If so, perhaps we want to ifdef it out, or something.

Comment 60

[Jason Sonnenschein]

(In reply to comment #59)
> Is Gecko's staggering actually disabled? Is it possible for them to interact,
> possibly with undesired results?
> 

No it's not disabled, but the values passed back are either exactly what was sent by gecko (if it's the first toplevel created since the program was run,) or those values are replaced entirely.

> 
> Change this to a printf instead.
> 

OK

> Remove the space between the colon and the argument, I'd say.
>

OK
 
> 
> Maybe it'd be better to use the cocoaRectToGeckoRect function, instead of
> repeating the logic over and over.
> 

OK

> Also, I'm not sure about that cast, I think the prevailing style is a bit
> different.
> 

I'm not sure what you mean, sorry.  Could you give me more detail?  I rarely code in C variants.

> 
> This is a better patch, but I'm curious to know if the gecko staggering will
> still happen. If so, perhaps we want to ifdef it out, or something.
> 

If we're going to muck with the platform independent files, then I'll just replace the stagger call to a call in this file instead (rather than doing our stagger calculation in constrainPosition.)

Comment 61

[Colin Barrett [:cbarrett]]

(In reply to comment #60)
> (In reply to comment #59)
> > Is Gecko's staggering actually disabled? Is it possible for them to interact,
> > possibly with undesired results?
> > 
>
> No it's not disabled, but the values passed back are either exactly what was
> sent by gecko (if it's the first toplevel created since the program was run,)
> or those values are replaced entirely.

Hm, it's probably OK then?

> I'm not sure what you mean, sorry.  Could you give me more detail?  I rarely
> code in C variants.

(int)3 or (int)(3) instead of int(3), since int() isn't actually a function, you're casting. Style is up to the module owner in the end though, so that's more just my personal suggestion.

> > This is a better patch, but I'm curious to know if the gecko staggering will
> > still happen. If so, perhaps we want to ifdef it out, or something.
>
> If we're going to muck with the platform independent files, then I'll just
> replace the stagger call to a call in this file instead (rather than doing our
> stagger calculation in constrainPosition.)

That might not be possible for a couple of reasons, but that's in another module, so we'll see.

Comment 62

[Jason Sonnenschein]

I see the method now to bypass gecko stagger entirely.  There's already an ifdef for linux to do this.  I'll just piggy-back on that and add the window stagger calls into the window create routine.  It's a really clean way to do it.  

Thanks for the cast explanation.

Comment 63

[Josh Aas]

Comment on attachment 267030 [details] [diff] [review]
Second proposed fix against cvs 2007-05-26

It seems to me that putting cascading logic inside of ConstrainPosition is wrong. That is a general function, it isn't there for the specific purpose of window cascades.

I think we need to fix the cascading logic in the DOM window and if that is right and we're feeding it bad information from widgets then we need to solve that problem.

I'm not saying the problem isn't with an unimplemented ConstrainPosition (it well could be), but ConstrainPosition needs to be implemented correctly in the generic way intended, not this way.

Comment 64

[Jason Sonnenschein]

Josh: 

Is there another place in the code that you think it would be acceptable to use cocoa staggering routines?  I think they do a better job than the gecko ones, and X-windows *nix variants already bypass the gecko staggering in favor of the window manager's cascading routines.

If you do want to stick with the gecko routines, it seems to me that we're either passing bad screen size, or bad window height.... those are the main inputs that the stagger code is using, but there are other routines using screen size that seem to use it correctly.  The window height routine looks much like that of the other OS's and is used by resize routines as well.

Your Thoughts?

Comment 65

[Jason Sonnenschein]

Josh: 

Is there another place in the code that you think it would be acceptable to use
cocoa staggering routines?  I think they do a better job than the gecko ones,
and X-windows *nix variants already bypass the gecko staggering in favor of the
window manager's cascading routines.

If you do want to stick with the gecko routines, it seems to me that we're
either passing bad screen size, or bad window height.... those are the main
inputs that the stagger code is using, but there are other routines using
screen size that seem to use it correctly.  The window height routine looks
much like that of the other OS's and is used by resize routines as well.

Your Thoughts?

Comment 66

[Jason Sonnenschein]

I don't have more time to work on this for now.  So... just to update all of what I found.

I couldn't track down the actual bug...

Gecko's stagger code is platform independent, so Windows should have problems, too, if the stagger code is bad.  Linux doesn't use the stagger code.  It bypasses it.

Altering the "Get Available Rect" code returned to the stagger code fixes windows going behind the dock, but shortens menus

Altering the window height returned to the stagger code not only didn't look like the right thing to do, but also breaks staggering... didn't look into why.

I could get things working nicely by having cocoa "cascading" decide where the window should be... if it runs _after_ the gecko stagger code runs.

Disabling the gecko stagger code entirely and trying to use cocoa cascading in the create window routine didn't work.  I think that there's some window-manager initialization that happens in the gecko stagger routine that has to be done before the cocoa cascading routines will work.  

If Josh chimes in that it's OK to use cocoa cascading routines in "create window" rather than the "constrain position" routines, then it would be worth tracking down what needs to be initialized for the cocoa stuff to work, then just put it all in the gecko window creation routines.

Comment 67

[Jeremy]

(In reply to comment #66)
> I don't have more time to work on this for now.  So... just to update all of
> what I found.
> 
> I couldn't track down the actual bug...
> 
> Gecko's stagger code is platform independent, so Windows should have problems,
> too, if the stagger code is bad.  Linux doesn't use the stagger code.  It
> bypasses it.
> 

Not sure how related this is, but note that Bug 368023 is a cross-platform problem with the size and position of new windows, so I think it's related and maybe the cocoa widget is not to blame for this bug.  On the other hand, Camino 1.5.1 and SeaMonkey 1.1.4 don't have this problem, so I don't think it's a Gecko bug either.  I don't really know, I haven't delved into any of the code, I'm just mentioning it in case this helps someone to see a way to fix this bug and Bug 368023 at the same time.

Comment 68

[Jeremy]

Oops, sorry, I guess the current version of this bug IS in SeaMonkey.  I just reproduced it with the nightly build:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.7pre) Gecko/20070903 SeaMonkey/1.1.5pre

The bottom part of the window (below the HTML content rendering, the part with the resize box) can be positioned off-screen when the dock is hidden.  But I was only able to reproduce that on a Mac OS X 10.3.9 machine, not a Win2K machine.

Comment 69

[Jason Sonnenschein]

Attachment: Third proposed fix (against CVS 2007-09-06)

Well, if the bug is in the non Mac-specific code, then there's an easy fix.  The XUL staggering code is just taking the Mac menubar into account twice.  This patch is simple and fixes it.  It affects no other OSes and affects no other use of screen dimensions nor window dimensions.  I verified it works both when the dock is at the bottom of the screen, and the side.

Dev's please review the following patch.

Comment 70

[Amir Caspi]

Jason,

Does your patch fix the left-right cycling/stagger issue as well?  As mentioned in comment #38, the behavior on Linux is correct (new windows cycle back to the far left when the top window hits the right edge of the screen), while on Windows the window tiling starts reversing from right to left.  On the Mac, the first new window after hitting the right edge tiles down, but does not stagger either left or right.  Subsequent windows do a reverse-stagger from right to left.

IMHO the behavior under Linux is correct (and mimics the way Camino, Netscape, and even Safari work on the Mac).

Comment 71

[Jason Sonnenschein]

Hello Amir,

I don't see the behavior you talk about in comment 38.  

On my mac I see the same behavior for tiling as you described for Windows machines.  This is expected.  Windows and Mac are using "stagger" code written into Gecko to decide window placement.  Linux bypasses the code and lets the window manager handle it.  So you'll never see the same behavior between Linux and Mac OS X because OS X isn't using X11 window managers to tile winodws.

Comment 72

[Jason Sonnenschein]

FYI... Making Firefox tile like Safari or Camino would mean not using Gecko staggering, and asking Aqua to use its native tiling, instead.  If that's something people want to do, we'd need direction from Josh as to where to put it in the code

Also FYI:  I found a possible cause for the bug.  GetScreenBounds returns the X and Y of the window with titlebar of the window included, but it returns height and width of the content rect (on all OSes as far as I can tell.)  If the people who wrote the stagger code forgot this, then we'd see the behavior we're seeing.

Comment 73

[Josh Aas]

"asking Aqua to use its native tiling, instead.  If that's something people want to do, we'd need direction from Josh as to where to put it in the code"

Yeah, we don't have a good way to inject native tiling algorithms at this point. I'll check out the posted patch first and see if that improves the situation some, then we can look into how we might do native tiling next.

Comment 74

[Amir Caspi]

(In reply to comment #71)
> On my mac I see the same behavior for tiling as you described for Windows
> machines.  This is expected.

I was a bit wrong in comment #38; I revised my description of the behavior in comment #70, namely that the first window after hitting the right edge tiles directly down (and does not stagger back to the left), while subsequent windows will stagger back to the left (as is done in Windows).  So they're not entirely identical, but close.

As for using native tiling, that would be really great.  Perhaps the code to do that can just be ripped out of Camino?  I think native tiling would be a great benefit because it makes the app behave much more consistently given the OS (i.e. it behaves like other native apps).  Whatever can be done would be awesome - I'm putting in a vote for it. =)

Comment 75

[C.S. Mo]

I think it would be really great if it could use native tiling... Nice to see that, after 4 years this is getting attention. I'm still using Camino, despite some interesting advances in FireFox specifically because the tiling behavior in Camino is much nicer.

Comment 76

[Jeremy]

Using the native window manager would be great and might solve several other bugs (such as Bug 251024, Bug 323469, and Bug 354187) as well.

Comment 77

[Josh Aas]

Comment on attachment 280026 [details] [diff] [review]
Third proposed fix (against CVS 2007-09-06)

I don't see how this patch could possibly work. Have you tried this with different multiple monitor configurations? What if the window is not on the main screen, so its screen top is not 0,0? "screenBottom" is just calculating the bottom coordinate for the screen the window is on, which has nothing to do with the menu bar and should work fine on mac.

Comment 78

[Josh Aas]

Attachment: josh fix v1.0

This fixes everything for me. We had a silly cycle of bad code confusing things. The root of the problem is that our implementation of nsIWidget's GetScreenBounds didn't do the same thing it does on Windows and Linux. There is no need for us to do it any differently. Firefox's browser window code compensated in a bad way for that problem, I also removed that compensation to fix window size on startup.

Comment 79

[Colin Barrett [:cbarrett]]

Comment on attachment 280675 [details] [diff] [review]
josh fix v1.0

Getting rid of comments with "I have no idea why" in them is awesome.

Comment 80

[Josh Aas]

Attachment: josh fix v2.0

With my last patch I noticed another problem that led to a further fix. Under certain conditions new windows would be larger than the last new window. This includes an additional for that problem. Basically there are 3 positioning/sizing bugs between browser and widget code that compensated and covered up for each other in the past, this unravels that and I can't find any more bugs at all. Nice to see this gone after so many years!

Comment 81

[Josh Aas]

I should add back the information about |[mWindow isSheet]| vs. |mWindowType == eWindowType_sheet| somewhere. That is useful information even if it isn't relevant there any more.

Comment 82

[Asaf Romano (gone)]

Comment on attachment 280712 [details] [diff] [review]
josh fix v2.0

r=mano

Comment 83

[Josh Aas]

landed "josh fix v2.0" on trunk with a comment about testing for sheetiness re: comment #81

Comment 84

[Amir Caspi]

Josh, I noticed this is now marked as "fixed."  Thanks for fixing it! =)  Would you consider native tiling (a la Camino or the Linux Firefox) to be a separate bug, requiring a separate bug report?

Comment 85

[Josh Aas]

Definitely a separate bug report, make sure there isn't one filed already.

Comment 86

[Jeremy]

Josh, thank you very much for fixing this bug!  Looking over what you did it looks to me like you're on a roll to fix Bug 394843 and Bug 368023.  

I notice that StaggerPosition() does not check to see if the *requested* position is on screen, it only ensures that any *adjusted* position is on screen.  I can't find documentation on what StaggerPosition's "contract" is, but perhaps it should be considered part of the guarantee that the returned position is on screen whether or not the requested position is on screen.

Of course, I notice that after the call to StaggerPosition there is a call to mWindow->ConstrainPosition().  But I also notice that the implementation of ConstrainPosition in mozilla/widget/src/cocoa/nsCocoaWindow.mm is empty! So I think at least one of the two functions should be fixed to force the window onscreen.  I think mozilla/widget/src/windows/nsWindow.cpp provides a good example implementation of ConstrainPosition and that should probably be filled in anyway.  It looks pretty easy but I'm not a Mac programmer and I don't know Objective-C or Cocoa so I can't do it.  

Whatever you do, please update Bug 394843 and Bug 368023 with your recommendation of what to do or description of what you did to address those bugs.

Comment 87

[Dave Townsend [:mossop]]

Could this have caused bug 396345?

Comment 88

[Amir Caspi]

(In reply to comment #85)
> Definitely a separate bug report, make sure there isn't one filed already.

Just to follow up, I just opened Bug #421749 for native tiling.

Comment 89

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008030504 Minefield/3.0b5pre ID:2008030504

This should have been fixed in beta1 as far as I can see. If it's wrong, please correct.

Comment 90

[Fred]

If this bug is deemed "fixed" then I guess the bug I'm trying to get fixed is a different bug. If someone could address this and perhaps point me to a different established bug, or advise me to open a new bug report, I'd be grateful.

To repeat: my issue is that the bottom of subsequently opened windows fall off the bottom of my display screen, and I can't see the status bar which shows link URLs, etc.

Upon launching Firefox, I always expand the main window to full-screen size by
dragging its lower right corner (for some reason, clicking the green button
only toggles between two differently sized windows both of which are less than
full-screen ... in many other Mac apps that button will expand the window to
full-screen). And then subsequent windows open as full screen as well,
cascading vertically just below the first window's title bar, but not cascading
to the right. Each additional window opens exactly over the second window, that
is, they don't continue to cascade down below the previous window's title bar.

Anyway, when I open a second window over a first full-screen window, the top of this second window cascades just below the first, which is OK. However, its bottom can't be seen below the bottom of my 19" display screen ... can't see the
status bar, etc.. This is incorrect; it should resize its bottom so that it
doesn't go past the bottom of the screen. Otherwise, I'm forced to grab the
whole window, move it up, grab its bottom, move that up, then move the window
down again so that its top cascades under the first window ... lots of
jockeying back and forth, very tedious.

Opening a third (or fourth, etc.) window has the same problem as the second,
and they sit right on top of the second (rather than continuing to cascade
their window tops).

Again, I'd really appreciate someone addressing the very specific scenario I'm outlining here, because I first reported this two years ago when I was using Firefox 1.5, and now I'm using Firefox 2.0.0.12 on Mac OS 10.3.9 and it's still a big problem.

Thanks,
Fred

Comment 91

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #90)
> outlining here, because I first reported this two years ago when I was using
> Firefox 1.5, and now I'm using Firefox 2.0.0.12 on Mac OS 10.3.9 and it's still
> a big problem.

Fred, this missing feature you pointed out is and will not be fixed for Firefox 2.0.0.x. It is fixed in the upcoming Firefox 3. You could grab the latest beta 3 release and give it a shot. Don't forget to create a new test profile when doing that because not all bugs are fixed while switching between Firefox 2 and Firefox 3 forth and back.

Comment 92

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #91)
> (In reply to comment #90)
> > outlining here, because I first reported this two years ago when I was using
> > Firefox 1.5, and now I'm using Firefox 2.0.0.12 on Mac OS 10.3.9 and it's still
> > a big problem.
> 
> 2.0.0.x. It is fixed in the upcoming Firefox 3. You could grab the latest beta
> 3 release and give it a shot. Don't forget to create a new test profile when

Sorry, but I checked it too late. Firefox 3 doesn't support OS X 10.3.x any longer. You need at least 10.4 to be able to run it.

Comment 93

[Fred]

Thanks for the promising news, Henrik.

As long as it's fixed in Firefox 3 I can wait. I'm almost ready to install 10.4 anyway ... I like to stay at least one system behind the curve!

Any rough idea when Firefox 3 will be final?

Thanks,
Fred


(In reply to comment #92)
> (In reply to comment #91)
> > (In reply to comment #90)
> > > outlining here, because I first reported this two years ago when I was using
> > > Firefox 1.5, and now I'm using Firefox 2.0.0.12 on Mac OS 10.3.9 and it's still
> > > a big problem.
> > 
> > 2.0.0.x. It is fixed in the upcoming Firefox 3. You could grab the latest beta
> > 3 release and give it a shot. Don't forget to create a new test profile when
> 
> Sorry, but I checked it too late. Firefox 3 doesn't support OS X 10.3.x any
> longer. You need at least 10.4 to be able to run it.
> 

Comment 94

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #93)
> Any rough idea when Firefox 3 will be final?

When it's done. You can see the current status here:
http://wiki.mozilla.org/WeeklyUpdates/2008-03-10