Closed Bug 239876 Opened 21 years ago Closed 21 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.7final

People

(Reporter: shy, Assigned: danm.moz)

References

Details

(Keywords: fixed1.7)

Attachments

(2 files, 1 obsolete file)

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.
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.
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.
Assignee: general → danm-moz
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
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.
Oh. This will also fix bug 118717 and I believe bug 183633.
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.
Attachment #146118 - Flags: superreview?(adamlock)
Attachment #146118 - Flags: review?(jag)
Blocks: 240381
Blocks: 176342
Comment on attachment 146118 [details] [diff] [review] rewrite of final window sizing and placement awww Adam's not a sooper reviewer
Attachment #146118 - Flags: superreview?(adamlock) → superreview?(blizzard)
Attachment #146118 - Flags: superreview?(blizzard) → superreview+
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);
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).
Attachment #146118 - Attachment is obsolete: true
Attachment #146118 - Flags: superreview+
Attachment #146118 - Flags: review?(jag)
Attachment #146801 - Flags: superreview?(blizzard)
Attachment #146801 - Flags: review?(jag)
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
Attachment #146801 - Flags: review?(jag) → review+
This was checked in yesterday morning, and I've heard no complaints about new window sizing bugs. Closing fixed.
Blocks: 118717, 183633
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8alpha
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.
Attachment #146801 - Flags: approval1.7?
Comment on attachment 146801 [details] [diff] [review] rewrite of window sizing and placement a=chofmann for 1.7
Attachment #146801 - Flags: approval1.7? → approval1.7+
Now appearing on the 1.7 branch too.
Target Milestone: mozilla1.8alpha → mozilla1.7final
Keywords: fixed1.7
Attachment #146801 - Flags: superreview?(blizzard)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: