91137 - Window does not always have the correct height when no height param is mentioned in the properties

Status: NEW

(Core :: XUL, defect)

Description

[Rangan Sen]

Below is an example xul script for a window, that would nest some boxes inside 
it. No height param was mentioned in the windows properties. So, it might be 
expected that the window's height would be just sufficient to accomodate all the 
boxes insiade it.But what happens is that the window's height is much more that 
is actually needed, leaving a lot of space below the OK/Cancel buttons. I have 
not included any stylesheet here; and the script can be used alone.

----Here is the example script -----------------
<?xml version="1.0"?>

<window 
   id="test" 
   title="Test Title" 
   width="460" 
   xmlns:html="http://www.w3.org/1999/xhtml"
   xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"      
>

<vbox style="margin: 5px;">

  <vbox >
    <html>Testing autosizing of windows</html>
  </vbox>

  <hbox orient = "vertical">
    <html id="id1"/>
    <button id="Test1" label="Test1" style="width: 8ex"/>
    <button id="Test2" label="Test2" style="width: 8ex"/>
    <button id="Test3" label="Test3" style="width: 8ex"/>
  </hbox>
  
  <vbox >
    <html>Test Message</html>
    <grid>
      <columns>
        <column />
        <column />
      </columns>
      <rows>
        <row>
          <button id="cert" label="Test A" style="width: 8ex"/> 
        </row>
        <row>
          <button id="view" label="Test B" style="width: 8ex"/> 
        </row>
      </rows>
    </grid>
  </vbox>
  
  <hbox align="center">
    <button id="ok" label="OK" style="width: 8ex" /> 
    <button id="cancel" label="Cancel" style="width: 8ex" /> 
    <button id="help" label="Help" style="width: 8ex" /> 
  </hbox>

</vbox>
</window>

Comment 1

[Mike Pinkerton (not reading bugmail)]

over to jag in danm's absence. jag, what should be done with this? is it valid?

Comment 2

[jag (Peter Annema)]

Attachment: [testcase] simplified testcase

Comment 3

[John Morrison]

So, it looks like if no height is set, then the height is made equal to the 
width (which is "an other" expected behaviour).

Comment 4

[jag (Peter Annema)]

But only if the width is set. If you remove the width attribute, the window will
intrinsically size. Both horizontal and vertical intrinsical sizing is now
controlled by one boolean, perhaps we should split that up.

It looks like pres shell supports that, it's just that higher up we don't. I
don't think it's trivial to add support for this, but not extremely hard either.

Maybe add  void sizeToContentHorizontally() and void sizeToContentVertically(),
so we can call those where we now call sizeToContent() in
LoadPositionAndSizeFromXUL?

Comment 5

[jag (Peter Annema)]

dr, she's all yours.

Comment 6

[Dan Rosen]

ARRR!!!

Comment 7

[Rangan Sen]

Also, with the width set, and using very large fonts, the html messages inside 
the boz sometimes doesnot wrap around properly, and gets chopped off. A strange 
workaround like this resolves the problem, but I don't know how..

var xulWindow = document.getElementById("serverCertExpired");

var wdth = window.innerWidth; // THIS IS NEEDED,
window.sizeToContent();
xulWindow.setAttribute("width",window.innerWidth + 30);

var hght = window.innerHeight; // THIS IS NEEDED,
window.sizeToContent();
xulWindow.setAttribute("height",window.innerHeight + 40);
  

Comment 8

[Håkan Waara]

The workaround described by Rangansen is very ugly, and is now being
widespreadly used.

I think this should have higher priority than it currently does.

Comment 9

[Dan Rosen]

bumping up to 0.9.4 -- i agree this is higher priority... jag, you had an idea
with this?

Comment 10

[jag (Peter Annema)]

Well yeah. Right now we only size intrinsically if neither height nor width is
set. What we want to do is split that up into horizontal and vertical intrinsic
sizing, and do what's needed depending on whether width, height, both or neither
are set. If you look at how the code is now, extrapolating the split-up case
shouldn't be too hard, though it's something that will affect some interfaces
and may need some patching of code here and there. This lxr url should give you
an idea of where to start looking:

http://lxr.mozilla.org/seamonkey/search?string=mIntrinsically

So you'd end up adding two routines all the way down to nsIMarkupDocumentViewer,
similar to sizeToContent, one for horizontal and one for vertical sizeToContent,
which would take a parameter for height and width respectively (and pass that on
to ResizeReflow):

http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDocumentViewer.cpp#5014

Or something like that :-)

Comment 11

[Håkan Waara]

FYI, I've *never* gotten intrinsic sizing of dialogs to work. I did some
testcases the other day under a heated debate about window sizes with
BenGoodger; no luck.

Please fix intrinsic sizing so it works (at all), while you're on it. ;)

Thanks!

Comment 12

[Dan Rosen]

[spam] -> 0.9.5

Comment 13

[Dan Rosen]

[spam] [e-mail]'s bugs subject to redistribution by chofmann. R!

Comment 14

[Kai Engert [:KaiE:]]

I've done some experiments, see also bug 124241. On Linux I only see this
problem, if WIDTH is defined like "400", i.e. if it is a absolute number of pixels.

I don't see a problem, if I use style="width: 40em;" instead (for example), even
if I only specify a width (no height). With this change, I was able to get rid
of the described hack, and dialog sizes still looked good.

How are chances this will work on all platforms?

Comment 15

[Mox Soini]

Works for me on Linux and Windows (related to the patch in bug 124241)

I've tested on builds around 2.-9. of April 2002 with both windows (small and
large font sets) and linux and they work fine.

Also works when adding the following to userChrome.css (was a testcase)

dialog, button {
  font-size: 5.0mm !important;
  font-family: helvetica !important;
}

... So everythings great ! Thanks :)

Please take care of the bugzilla's bugs that are related to this (close, mark
fixed or something, if applicable):

bug 94384, bug 76248

and some more-or-less-related: bug 92410, bug 94905 (has sub-bugs), bug 89420

Comment 16

[[e-mail]]

Attachment: Proposed patch (-w version)

This patch simply makes windows require both width and height before they are considered to be extrinsically sized.

Note that if you only specify the width then this is now the inner width; if you specify the width and height then they are outer widths and heights (this makes a difference on those platforms where we know the size of the OS chrome).

Comment 17

[BugBot [:suhaib / :marco]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.