When an outer table is flowed at some computed width and height, it does not simply pass these values through to the inner table. Instead it tries to pull values off the table tag etc. etc. and thus ends up blowing away the computed width and height passed in. For the case where the computed width and height are not unconstrained, the outer table should not be replacing these values. It should be relying on them. This is preventing tree widgets and tables from working inside boxes, and according to Troy is a bug in the table code.
David, please supply a test case, preferably one without apprunner, xul, etc.
Hmmm. I'm not sure how to supply a test case in HTML. I can give you a XUL test case.
I think an XML test case would be acceptable as well. Not a XUL test case, though. :-)
Here is a XUL test case (I needed to use XUL to get support for inline style, because I think this makes it easier for you to see what's going on visually). ====== <?xml version="1.0"?> <!-- -*- Mode: SGML; indent-tabs-mode: nil; -*- --> <?xml-stylesheet href="chrome://global/skin/xul.css" type="text/css"?> <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" xmlns:html="http://www.w3.org/TR/REC-html40" align="vertical" style="width: 300px"> <box style="height:300px; border: 3px solid blue" align="vertical"> <html:table flex="1" style="height:100px; border: thin solid red;"> <html:tr><html:td>Row 1 of 3</html:td></html:tr> <html:tr><html:td>Row 2 of 3</html:td></html:tr> <html:tr><html:td>Row 3 of 3</html:td></html:tr> </html:table> </box> </window> ====== What you'll see: a 300 by 300 blue box that contains a much smaller (red-bordered) table. The table was actually being flowed with a computed width and height of 300 by 300. Notice that the table doesn't stretch itself horizontally, despite being told that it's computed width is wider. Notice more importantly that the table doesn't stretch itself vertically. It instead sized itself to 100 pixels, matching the style rule on the table. This is wrong, since the parent box is the one that chose to ignore that style rule and flow the child at a larger computed height. My hope is that this is easy to fix. I believe it's just that the inner table frame is looking at style rules on the outer table frame or something, when it really should just be getting a pass-through (minus the table caption of course).
Err that should have read "its computed width is wider"... the grammar police will be coming for me soon.
I believe this situation really is unique to XUL, since in HTML, the parent container would typically have used and honored the style request to be 100 pixels tall. Boxes are different in that they can stretch content, even when a style rule specifying a size is present. The box in this test case looks at the 100 pixel height request and then sees that the flex attribute is present. Because it's there, the box knows that the table should be stretched to be 300 pixels instead, and so it passes that in as the computed height. Somewhere along the line, though, the table is looking at the 100 pixel height request and using that instead. So your mission, should you choose to accept it, is to get the red bordered box to stretch so that it's as big as the blue bordered box.
Maybe you can use min-height and max-height to help get what you want? Something like "let the computed height be shrink wrap, but make sure the height is at least xxx and no more than yyy"
I'd like to avoid special-casing the box code when dealing with trees and tables. IMO what the box code is doing is quite reasonable, and I suspect this bug is pretty easy to fix (i.e., that the style values are being pulled off by the table, when it has no business doing so).
It's true that looking at it now it has no business doing that, but keep in mind that the table code was written long before we added in things like nsHTMLReflowState I wasn't proposing min-height and max-height as alternatives to cleaning up the table code, I was simply mentioning them as options in controlling the computed width and height
And I might add hyatt, I implemented min-width, max-width, min-height, max-height support _just_ for the xpfe (and xul) folks. So, I would appreciate it if you would use it, somewhere, anywhere... :-)
Troy, we use it all over the place. You are loved. :)
I checked in a fix. It looks cool to me. If it causes any problems, I'll be ready to back it out pronto.
David, I am going to back out your changes since they broke a lot of the regression tests (located in layout/html/tests/table and invoked by "rtest baseline" before and "rtest verify" after). I'll keep your changes in as comments and will take the bug.
Ok, all yours.
David, this is fixed except for a minor thing you need to do. The box frame (unlike various html elements) is setting the computed width/height for the outer table frame as the content dimensions. For tables (and form control elements, I think) the computed values should be the box dimensions (including border and padding), not content dimensions. Fiddle with the table's border and padding in your example to see what I mean.
Hmm.. In everything I have encountered so far including form elements the computed size for a block or inline-replaced (like for elements) is the content area and does not include border or padding. It is up to the frame to return the computed size + it's border and padding. Take a look at the implementation of nsLeafFrame....
For Nav4.6 and IE5 compatibility, computed widths for form elements and tables include border and padding (unlike every other html element). When you say <table border width=100>, you get a table that is 100 pixels wide and that includes border and padding. When you say <div style="width:100px; border:1px solid black;"> you get a div whos content area is 100 pixels wide and doesn't include border and padding. You won't find this distinction in the CSS2 spec, but Peter has been active on the working group and has implemented a property which tells which edges to use to define the computed width. I'm not sure what the property is or how to use it yet, but it sounds like it should be used for this bug. You are right that in all cases, it is up to a frame's Reflow to return its desired size which includes border and padding. The only issue in question is how to interpret the computed values set on the ReflowState, and that is the point I am trying to make.
The CSS3 proprty in quesiton is called: 'box-sizing'. It determines which edge of a box the width & height properties apply to: content, padding or border. To make this work right, the HTML attribute mapping code (nsHTMLTableElement::MapAttributesInto) for 'width' on tables should *also* set the box-sizing property to 'border-box'.
So it sounds like the table needs to look at that property to see how it should interpet the computed size. So this bug should probably be reopened until that is fixed. Otherwise the the table will return the wrong size and will cause boxes to do very bad things. Boxes expect their children to return the computed size + their border and padding. If they don't it assumes they could not be sized and gives them a min size of what was returned.. The end effect will be that tables will just continously grow.
David H: Could you please verify this is fixed. I'm not familiar with 'box-style' or table 'flex'. If you agreed it's fixed, please verify it so. Thanks
David: Could you please verify this bug fixed if you agree? Thanks
Nope. This is not fixed. It should be reopened.
Reopened per reporter's comments
Adding rickg to cc list....need to investigate for PDT+ or -
I talked to Hyatt and he doesn't think this is PDT+.
Putting on PDT- radar, per karnaze comment.
Fixed with latest checkin.
David: Could I ask you to look at this again and see if it's fixed? Karnaze marked it fixed as of 1/2. Thanks
Hyatt, I noticed some changes you checked in to nsTreeOuterFrame that leads me to believe you don't think this got fixed. My changes fixed the example given. I forgot to mention that tables now have a default box-sizing property of border-box (compatable with Nav and IE) and respect box-sizing. If it is desired to use content-box, then that should be specified in the style.
Tables still ignore mComputedWidth and mComputedHeight on an incremental reflow targeted at the table. I added a hacked method called "FixBadReflowState" to tables (that only trees implement), so that trees would at least work. You can grep for this method to see where the problem is.
Should this bug be reopened?
yes, but if you do reopen it, remove the DOGFOOD and BLOCKER designations. It's not that serious anymore. It's almost totally fixed.
Removing DOGFOOD/BLOCKER designations and reopening bug per David's comments.
Moving to M14.
Removing PDT annotation now that dogfood nomination has been removed.
Moving to M16.
Tables have stopped using FixBadReflowState and now they handle mComputed values better. I forgot to remove the method, but I will at some point.
Verifying bug fixed. If reporter does not agree, please reopen.
Reopening to mark invalid.
I have removed the code that fixes this bug to fix topembed bug 106158 and clean up the table code, since evaughan assures me that a box can no longer contain a table. The outer table is a combination of the inner table and caption and always has auto width and height. It was only made to honor artifical mComputed value to accomodate boxes when trees used tables.
Marking verified. Invalid