356742 - [cocoa] Sheets are offscreen if opened when all windows closed (attached to hiddenwindow)

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Stefan [:stefanh]]

SeaMonkey recent trunk, mac os x.

SeaMonkey Open Web Location dialog is a sheet. If you close all windows and open the dialog it will pop out at the upper left corner with only the bottom visible. 

This wasn't the case when we where using mac widget code. If you opened the dialog when all your windows where closed, the dialog opened in form of a standard non-sheet dialog box in the upper left corner just below the menubar.

Comment 1

[Stefan [:stefanh]]

Attachment: Screenshot of misplaced Open Web Location sheet

To reproduce this:

1) Open SeaMonkey
2) Close the Seamonkey window
3) Hit Cmd+Shift+L

I managed to get the dialog completely visible one time. It looked like a sheet to me. So maybe the cocoa code doesn't handle cases like this (sheets opened when only hiddenwindow is open)?

Comment 2

[Stefan [:stefanh]]

If you make a js alert (sheet) open when no windows are open you'll see the same issue. Could it be that the sheet draws from the hidden windows titlebar - somewhere "above" your screen?

Comment 3

[Jesse Ruderman]

This might be the same as bug 424266.

Comment 4

[Stefan [:stefanh]]

This seems to works for me now (Leopard, recent trunk). Karsten, does it works for you with Tiger?

Comment 5

[Stefan [:stefanh]]

This have gotten worse on Leopard, so it's not fixed. What probably happens is that the sheet opens somewhere outside the screen.

Comment 6

[Stefan [:stefanh]]

Since the sheet is attached to the hiddenwindow it will open way offscreen. In order to get out of the situation you can hit Esc which will close the non-visible sheet.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Presumably there's code somewhere to check and see if there are any windows, and if not to display an app-modal dialogue instead; can that code check just be modified to exclude the hidden window?

Comment 8

[Karsten Düsterloh]

Attachment: deny hidden window as parent

Comment 9

[Karsten Düsterloh]

Lost comment:
The patch just kills the parent reference to the hidden window when creating a new window. All other parameters like modal, centerscreen, etc. are left intact (but are probably converted just like they are now: modal sheets without a parent become a normal dialog, etc.).

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 559281 [details] [diff] [review]
deny hidden window as parent

I'll need to test this patch -- I won't be able to understand all its
implications without doing that.

So please give me ways (hopefully several ways) to reproduce this bug
in Seamonkey and (if possible) also Firefox.

Testing in Seamonkey 2.4 on OS X 10.6.8, I currently just get a beep
when I follow the STR from comment #0.

Comment 11

[Stefan [:stefanh]]

I dunno about Firefox, but if it helps, it's easy to reproduce the issue in Thunderbird:

1) Launch a Thunderbird nightly
2) Close the window
3) From the File menu, choose New --> Address Book Contact…
4) Watch the contacts sheet open partially off-screen in the top left corner

Comment 12

[Steven Michaud [:smichaud] (Retired)]

That does help.  Thanks.

Comment 13

[Karsten Düsterloh]

Attachment: XPI showing the problem in FF (use Cmd-Q to exit)

This pseudo addon for FF just opens a dialog before any real windows are up.
The dialog will be centered upon the upper left corner of the screen.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 559281 [details] [diff] [review]
deny hidden window as parent

mnyromyr, thanks very much for your testcase (from comment #13).  But in my tests, your patch doesn't fix the problem that it demonstrates :-(

I tested (with a fresh profile) in a build made from current trunk code (and patched by your patch from comment #8) on OS X 10.6.8.

I haven't had a chance to do any digging, to figure out why your patch fails.  Hopefully I'll have time for that next week.

Comment 15

[Karsten Düsterloh]

FWIW, it doesn't work for me anymore either with current trunk (the comparison for the hidden window fails now).
I'll try to get a new patch up.

Comment 16

[Karsten Düsterloh]

Attachment: do ConstrainPosition

It took some time until I realized that window->ConstrainPosition didn't work, because it … is not implemented?!
This patch does not do any ambitious calculations, it just makes sure that the upper left corner of a window is not moved offscreen.

Comment 17

[Steven Michaud [:smichaud] (Retired)]

Attachment: Alternate implementation of ConstrainPosition

> It took some time until I realized that window->ConstrainPosition
> didn't work, because it … is not implemented?!

Amazing.

This seems to go back to the very earliest days of Cocoa widgets:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/cocoa/nsCocoaWindow.mm&rev=1.1
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/cocoa/nsCocoaWindow.mm&rev=1.30

I can't figure out why/how it stayed unimplemented for so long.  All
other platforms implement it except for something called "gonk"
(whatever that is).  Mac widgets (Cocoa widgets' predecessor) also
implemented it.

So it's high time we added support for this to Cocoa widgets :-)

But since our biggest platforms (Windows and GTK) also implement it,
and in the same way, I figure it's best to copy their implementations.
That's what my patch does.

In very brief testing it fixes this bug (as demonstrated by your
testcase).  Let me know if you can anticipate any problems using my
(more complex) patch instead of your (simpler) patch.

Comment 18

[Karsten Düsterloh]

Comment on attachment 578385 [details] [diff] [review]
Alternate implementation of ConstrainPosition

Looks good; works as expected for me in both FF and SM. 
Sorry for the long delay.

Comment 19

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 578385 [details] [diff] [review]
Alternate implementation of ConstrainPosition

Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/163c2800f87f

Thanks, Mnyromyr, for testing this and noticing the underlying bug (our lack of an implementation for nsCocoaWindow::ConstrainPosition())!

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/163c2800f87f