75649 - moveToAlertPosition is broken

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Simon Fraser [no longer active]]

moveToAlertPosition is broken in several ways:
1. It's often called with a window that has not been resized yet, so it gets the 
widths all wrong.
2. It does not prevent windows coming up above or to the left of the screen 
bounds.

Comment 1

[Simon Fraser [no longer active]]

Attachment: Fix in dialogOverlay.js

Comment 2

[Peter "jag" Annema]

I'm not sure you should be doing this in moveToAlertPosition(), since I could
call it after setting a hardcoded size, and it would break the API by having the
side-effect of resizing the window (something not obvious from the name of the
function).

Instead, place the sizeToContent() at those callers which need it, and see if we
can fix inner(Width|Height) and outer(Width|Height) to be defined if the
<window/> tag specifies a width and height...

Comment 3

[Simon Fraser [no longer active]]

Attachment: Better patch

Comment 4

[Simon Fraser [no longer active]]

Looking for alecf sr= love.

Comment 5

[Dan M]

You needing to fix js to keep a window onscreen implies that there's something 
wrong much deeper down. Security code that prevents JS from moving a window even 
partially offscreen should be taking care of this (this code is working on my 
(single-monitor) system, though on the Mac the window borders can leak a bit off 
the right and lower edges).

Comment 6

[Peter "jag" Annema]

Why would that security policy apply to chrome JS?

Comment 7

[Simon Fraser [no longer active]]

danm: I just fixed the code that keeps the window on screen because it was there 
already (added by srilatha in August 2000). I didn't fix it because I saw bad 
behaviour.

Questions for you, dan:
1. Why is the window sizeless when its onload is called? Shouldn't it have some
   kind of size already?
2. Why doesn't XUL have some kind of automatic 'default position' attribute,
   like the Mac OS's window positioning flags?
   There is currently no way to, automatically, get a dialog to default to the
   alert position, but have its position persisted.

Comment 8

[Dan M]

d'oh! wrong again, dan! Right: the security code won't kick in for chrome js.

1) An intrinsically sized window has an undefined size at the time of the js 
onload handler because it gets its size from another onload handler, and the 
order of event handler execution can't be guaranteed. Unless I'm wrong about 
that, too. Even if, we've had code before that depended on precise ordering of 
onload handlers that ended up breaking, so it's nothing to rely on. Basically, 
this sort of stuff you need to do in C.

2) Yes, that'd be a fine feature. I'd be totally pleased to have the time to 
implement it. I mean that. I'll mention it in related bug 50686, which is 
scheduled for 1.0.

Comment 9

[Peter "jag" Annema]

danm: from your comment under (1), does that mean that a window which has width
and height set in the xul window element, inner(Width|Height) and
outer(Width|Height) should be set at onload time regardless of the order of
onload handlers firing up? Or is that same onload handler taking care of that,
and are we basically screwed?

If we are screwed, can you think of a way to become unscrewed? My suggestion was
to force the caller of moveToAlertPosition() to either call sizeToContent() or
sizeTo(...,...) first, something we'd have to document at moveToAlertPosition()
when we take out the "assertion" dump (where did that latest patch go?).

Comment 10

[Alec Flett]

sr=alecf

Comment 11

[Alec Flett]

wait, I take that back, I didn't see the other comments

Comment 12

[Dan M]

  width and height specified in the <window> xul element are picked up in the 
same onload handler as is intrinsic sizing. With our current order of handler 
firing, that sizing should happen after the JS moveToAlertPosition function 
executes. We're a little bit screwed, but note that moveToAlertPosition is itself 
kind of a temporary hack until the fix in bug 50686 comes online. Being in JS, 
it's limited.
  Jag, I agree that the sizeTo... call should be in the function also calling 
moveTo... (Note that an intrinsically sized window will end up being sized twice, 
but the second time should give the same results.) Actually, looking at the code, 
I don't understand why a window's size set in the window.open(,,"height=") call 
manages to not be overridden by the setting in the <window> tag. So I'm unclear 
on something here. I'll have to look into that when I have the time.
  The moral is, these changes to prevent moveToAlertPosition putting the window 
offscreen seem worthwhile. Sizing the window intrinsically in JS will be 
inefficient, but probably acceptable for simple dialogs. Theoretically, windows 
sized by window.open() or intrinsically should work with this scheme (and I 
expect them to because window.open sizing is known to work, though suddenly I 
don't understand why), though there may (someday? now?) be problems with windows 
with persistent size. But I wouldn't spend a whole lot of time making 
moveToAlertPosition work perfectly.

Comment 13

[Simon Fraser [no longer active]]

I have an r= from jag, and sr=sspitzer.

Comment 14

[R.K.Aa.]

Is this fixed?
Downloading glibench.gz from url below displayed this in console:

Document http://www.ncpro.com/clibench/clibench.htm loaded successfully
Trying to position a sizeless window; caller should have called sizeToContent() 
or sizeTo(). See bug 75649.

Saw this today on a fresh CVS build, Linux.

Comment 15

[sairuh (rarely reading bugmail)]

thx to smfr for the test steps. only works on the inital browser window, so to
verify:

0. create a new profile.
1. launch browser, and move browser so that it's mostly offscreen.
2. bring up the Find dialog.

expected: Find dialog should appear over the browser window --but still *fully*
onscreen.

vrfy fixed on winnt, mac and even linux using 2001.05.29.0x comm bits.

Comment 16

[Fuzzy Gorilla]

Was this bug fixed/verified on the 0.9.1 branch or just the trunk?
I am using Mozilla 0.9.1 (Build Id: 2001060713) on Linux and see the following
messages appear in the window where I started Mozilla:

You have no mail.
Trying to position a sizeless window; caller should have called sizeToContent()
or sizeTo(). See bug 75649.

Should this bug be re-opened?

Comment 17

[sairuh (rarely reading bugmail)]

this was fixed before the branching, so both the branch and trunk should contain
the fix.

Comment 18

[Simon Fraser [no longer active]]

"Trying to position a sizeless window" warning are "normal", or at least not 
directly related to this bug.

Comment 19

[Phil Pishioneri]

Running linux build 2001061621, I just got this message:

Trying to position a sizeless window; caller should have called sizeToContent()
or sizeTo(). See bug 75649.

while trying to do a "Search Mail/News messages" from Mail.  Was attempting to
search the sender from an IMAP folder; error occurred when selecting the Search
button.  Duplicated it while searching the subject.

It also causes Mozilla to quit.

Comment 20

[Roland Mainz]

I am getting the same error as in comment #19 while converting a NS4.x profile
on Solaris ... ;-(

Comment 21

[Thomas Dodd]

This shouldn't be considered "normal"
Either the warning needs to be removed, or the
caller should us the functions listed in the warning.

Which is it?

Comment 22

[jag (Peter Annema)]

The warning is there for a reason. Guess what needs to be done? :-)

Comment 23

[Martin Mokrejs]

I just built mozilla-20031105 snapshot with debug enabled on Tru64Unxi and
browsed the http://www.gsf.de/ page. I got:

*** Chrome Registration of locale : Checking for contents.rdf at
jar:resource:/chrome/en-US.jar!/locale/en-US/messenger/
*** Chrome Registration of locale : Checking for contents.rdf at
jar:resource:/chrome/US.jar!/locale/US/messenger-region/
WARNING: dependent window created without a parent, file nsWindowCreator.cpp,
line 128
GFX: dpi=75 t2p=0.0526316 p2t=19 depth=24
we don't handle eBorderStyle_close yet... please fix me
WEBSHELL+ = 1
LoadPlugin()
/afs/.gsf.de/alpha_dux51/usr/mozilla-1.6a-20031105/lib/mozilla-1.6a/plugins/libnullplugin.so
returned 14044f6e0
GetMIMEDescription() returned "*:.*:All types"
Note: verifyreflow is disabled
Note: styleverifytree is disabled
Note: frameverifytree is disabled
WARNING: nsTimeoutImpl::Release() proceeding without context., file
nsGlobalWindow.cpp, line 5175
WEBSHELL- = 0
WARNING: dependent window created without a parent, file nsWindowCreator.cpp,
line 128
we don't handle eBorderStyle_close yet... please fix me
WEBSHELL+ = 1
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
nsFrame.cpp, line 2540
*** failed to select current profile in list
we don't handle eBorderStyle_close yet... please fix me
WEBSHELL+ = 2
WARNING: nsTimeoutImpl::Release() proceeding without context., file
nsGlobalWindow.cpp, line 5175
WEBSHELL- = 1
we don't handle eBorderStyle_close yet... please fix me
WEBSHELL+ = 2
WEBSHELL+ = 3
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
nsFrame.cpp, line 2540
Trying to position a sizeless window; caller should have called sizeToContent()
or sizeTo(). See bug 75649.
*** going to create a new profile called Default User in folder: /.mozilla
WEBSHELL- = 2
WEBSHELL- = 1
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsChromeRegistry.cpp,
line 3185
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsChromeRegistry.cpp,
line 3185
WEBSHELL- = 0
WEBSHELL+ = 1
WEBSHELL+ = 2
WEBSHELL+ = 3
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
nsFrame.cpp, line 2540
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
nsFrame.cpp, line 2540
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
nsFrame.cpp, line 2540
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
nsFrame.cpp, line 2540
Document http://www.mozilla.org/start/ loaded successfully
CSS Error (http://www.gsf.de/php-css/1.css :29.49): Error in parsing value for
property 'font-weight'.  Declaration dropped.
CSS Error (http://www.gsf.de/ :1.105): Error in parsing value for property
'width'.  Declaration dropped.


and many more. I'll attach the full output. Should I file a new bug or did this
bug re-appear? Thanks.

Comment 24

[Martin Mokrejs]

Attachment: output of mozilla when started and opened http://www.gsf.de

output of mozilla when started and opened http://www.gsf.de . That page used to
kill older mozillas(around version 1.0). So it's good we see the page at all,
but what to do with those warnings/errors? Ignore them?

Comment 25

[Dan M]

I agree the spate of warnings and occasional assertion Mozilla emits during
normal operations are annoying. Worrying, even. That could well be the subject
of a bug. However most of the warnings in your log seem to be legitimate
complaints about the quality of the HTML at gsf.de.

Is it the "trying to position a sizeless window" warning in particular that
you're worried about? Yes that's the Mozilla source complaining that someone is
misusing it. It's a largely self-correcting problem but I think still it's
worthy of a (new) bug report. In that report, please do mention what window is
being opened and how to open it. gsf.de doesn't seem to be attempting to open
any popup windows, and I sure don't get that warning when I drop in for a visit.

Comment 26

[Martin Mokrejs]

So I have logged bug http://bugzilla.mozilla.org/show_bug.cgi?id=224992. Thanks.

Comment 27

[ISHIKAWA, Chiaki]

Attachment: Console output that inludes the problematic lines.

the lines printed on the tty emulator where TB3.0.3 was invoked, and
that show the problematic lines:

Trying to position a sizeless window; caller should have called sizeToContent() or sizeTo(). See bug 75649.

Comment 28

[Daniel C]

Seeing this printed in Minefield, should this bug be moved and reopened, or a new one filed and the message changed?

Comment 29

[ISHIKAWA, Chiaki]

Hi,

I am afraid that this is still there, and I have found a way reliably to reproduce this.

Choose a message and click print priview from the context-menu (right click on the message and choose print privew.)

As soon as the very small "print priview" window (without any content) 
appears, I see the following message (in .xsession-errors under Gnome desktop under Debian GNU/Linux):

Trying to position a sizeless window; caller should have called sizeToContent() or sizeTo(). See bug 75649.

then a moment later, the window becomes larger to show the preview content (and I noticed that somehow the small window is again shown for a split second, but people who know the code can ascertain that such behavior will happen.),
and then I see the following two lines in ~/.xsesson-errors. Something is very fishy.:

Trying to position a sizeless window; caller should have called sizeToContent() or sizeTo(). See bug 75649.
ウィンドウ・マネージャーの警告: 0x3755d1b (印刷プ�) で指定したウィンドウ 0x3755b00 の WM_TRANSIENT_FOR が間違っています

The second line is a warning from the window manager (metacity of gnome desktop) and it is in Japanese: it translates roughly as

Warning from the Window manager: 0x3755d1b (Print P[review]; cut short due to the fixed size format display) has  specified (?) window 0x3755b00, but it has incorrect WM_TRASIENT_FOR.

The warning is terse and not for the ordinary users, and so its meaning is  obscure and I could not be sure of the translation, but 
I think it tries to tell the developers that usage of WM_TRANSIENT_FOR is wrong for this particular window associated with 0x3755d1b for "Print Preview".

Since I can reliably reproduce this every time by invoking "Print Preview", someone ought to look at this again.

Comment 30

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, chiaki from comment #29)

This bug is still there.
The message is still printed.

On a hunch, I checked what happens when firefox (on the same OS, Debian GNU/Linux)
is run. I checked the Print Prview of Firefox (v 20.0), but nothing happens.

So I think this problem is specific to Thunderbird code.

As Daniel Cater asked rhetorically(?) if this bug should be re-opened, and I read the older comments and decided that the subject line is quite not right for this particular
instance of misuse and so decided to file a new bug. TIA.