239876 - combined specification of one inner and one outer dimension of a popUp window is not honored

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Sebastian Winkler]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040317 Camino/0.7+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7b) Gecko/20040317 Camino/0.7+

With window.open() it's not possible to specify one dimension by an
inner-attribute and the other by an outer-attribute.
For example this does not work:
window.open('','','innerWidth=400,outerHeight=200');
Only the outer-value is honored. The inner-value is inherited from the opener.

I'm not sure if this is not so intentionally, but I think it shouldn't be like this.
One example where this functionality would be needed is when you want to display
a window with a fixed content width (=> innerWidth) with the maximum available
height (screen.availHeight => outerHeight).

Reproducible: Always
Steps to Reproduce:
1. Try the attachment I will include.
2.
3.

Actual Results:  
only outerWidht/outerHeight are respected.
innerWidth/innerHeight are inherited.

Expected Results:  
Both values should be respected, I think.

Comment 1

[Sebastian Winkler]

Attachment: Test case for combined specification of one inner and one outer dimension of a popUp window

Comment 2

[Boris Zbarsky [:bzbarsky]]

The code involved is at
http://lxr.mozilla.org/seamonkey/source/embedding/components/windowwatcher/src/nsWindowWatcher.cpp#1506

It looks like we should maybe use different booleans for sizeChrome in the X and
Y directions.

Comment 3

[Dan M]

Since an outer size isn't supported by IE and as I'm pretty certain this code
was lifted from Navigator 4.x or earlier, a mixed inner/outer size spec has
probably never worked in the history of the web, with the possible exception of
some of the more recent browsers. How's that for a run-on sentence?

It's obviously not a heavily requested capability, but it is pretty much a bug.
And the somewhat crufty new window sizing code could stand some attention. Note
that if this is fixable, so is bug 176342.

Comment 4

[Dan M]

Attachment: rewrite of final window sizing and placement

This patch fixes this bug, bug 176342 and part of bug 240381, and some bugs
that are unnamed as far as I know. For example, on multiple monitor systems the
current code will have trouble with windows given positions in the window.open
call.

This patch does force all new windows to be initially sized and positioned to
fit on a single screen. That was fallout from fixes for some of the above bugs,
but could be annoying to some users.

This patch also assumes that by the time this method is called, a window has
already been given a valid position and size. The current code doesn't. I
believe this is always a valid assumption in Mozilla. But this patch also
affects embedded environments and this has never been an explicit requirement
in the embedding API. However it's a reasonable assumption even there, since
this is the last thing done before the window is made visible, and there is no
guarantee this method will make any changes to the window's size or position.
Therefore, despite the lack of documentation, embedded apps have probably
always sized and positioned their windows by this point.

Comment 5

[Dan M]

Oh. This will also fix bug 118717 and I believe bug 183633.

Comment 6

[Dan M]

Comment on attachment 146118 [details] [diff] [review]
rewrite of final window sizing and placement

Requesting reviews from Jag and Adam, because while the code lives in the
embedding directory, it's mostly an xpfe issue. However there is also that
embedding assumption; see comment 4.

Comment 7

[Dan M]

Comment on attachment 146118 [details] [diff] [review]
rewrite of final window sizing and placement

awww Adam's not a sooper reviewer

Comment 8

[Dan M]

Small tweak:
+      if (height < 100)
+        height = 100;
+      if (height > screenHeight)
+        height = screenHeight;
+      if (width < 100)
+        width = 100;
+      if (width > screenWidth)
+        width = screenWidth;
becomes
+      if (height < 100)
+        height = 100;
+      if (winHeight > screenHeight)
+        height = screenHeight - (sizeChromeHeight ? 0 : chromeHeight);
+      if (width < 100)
+        width = 100;
+      if (winWidth > screenWidth)
+        width = screenWidth - (sizeChromeWidth ? 0 : chromeWidth);

Comment 9

[Dan M]

Attachment: rewrite of window sizing and placement

Found some bugs in the last version. This is like the previous one, but uses
the ScreenManager and nsIScreen instead of nsIDOMScreen (alerts return a broken
DOMScreen), and also does not force a size out-of-bounds check on windows for
which a size is not specified in the window.open |features| parameter. To do so
breaks intrinsically sized windows.

This new version won't be able to properly force an intrinsically sized window
entirely on-screen, but that hardly seems like a problem (most are small and
many are centered, anyway).

Comment 10

[jag (Peter Annema)]

Comment on attachment 146801 [details] [diff] [review]
rewrite of window sizing and placement

+  { nsCOMPtr<nsIBaseWindow> shellWindow(do_QueryInterface(aDocShellItem));

Perhaps split that out to two lines, with a comment like "limit visibility and
life time of shellWindow"

Other than that, looks good to me. r=jag

Comment 11

[Dan M]

This was checked in yesterday morning, and I've heard no complaints about new
window sizing bugs. Closing fixed.

Comment 12

[Dan M]

Comment on attachment 146801 [details] [diff] [review]
rewrite of window sizing and placement

Requesting for 1.7. This has been in the trunk for more than a week and I've
heard no complaints. It fixes a variety of niggling problems (this bug, bug
118717, bug 176342, bug 183633, bug 240381). Its main advantage is this: a user
may have a default window position off in a corner and not notice this because
he habitually browses with a maximized window. New unpositioned windows may
initially be positioned partially offscreen without this patch. See for example
bug 240381. This causes some confusion and annoyance; no one ever realizes what
the problem is.

Comment 13

[chris hofmann]

Comment on attachment 146801 [details] [diff] [review]
rewrite of window sizing and placement

a=chofmann for 1.7

Comment 14

[Dan M]

Now appearing on the 1.7 branch too.