Closed Bug 12287 Opened 25 years ago Closed 23 years ago

Tables ignore mComputedWidth and mComputedHeight

Categories

(Core :: Layout: Tables, defect, P3)

Product:
Core
Component:
Layout: Tables
Platform:
x86
Other
Type:
defect

Tracking

()

Status:
VERIFIED INVALID

People

(Reporter: hyatt, Assigned: karnaze)

References

Details

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.
Assignee: karnaze → hyatt
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. :-)
Assignee: hyatt → karnaze
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. :)
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
I checked in a fix.  It looks cool to me.  If it causes any problems, I'll be
ready to back it out pronto.
Status: RESOLVED → REOPENED
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.
Resolution: FIXED → ---
Status: REOPENED → NEW
Ok, all yours.
Status: NEW → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
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.
Depends on: 12012
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.
Whiteboard: 11/16: Requested verification by reporter
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
Whiteboard: 11/16: Requested verification by reporter → 12/3: Requested verification by reporter
David: Could you please verify this bug fixed if you agree? Thanks
Nope. This is not fixed.  It should be reopened.
Status: RESOLVED → REOPENED
Whiteboard: 12/3: Requested verification by reporter
Reopened per reporter's comments
Resolution: FIXED → ---
Adding rickg to cc list....need to investigate for PDT+ or -
I talked to Hyatt and he doesn't think this is PDT+.
Whiteboard: [PDT-]
Putting on PDT- radar, per karnaze comment.
Status: REOPENED → ASSIGNED
Target Milestone: M13
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Fixed with latest checkin.
Whiteboard: [PDT-] → [PDT-] 1/7: Pending verification from reporter
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.
Status: RESOLVED → REOPENED
Summary: [DOGFOOD/BLOCKER] Tables ignore mComputedWidth and mComputedHeight → Tables ignore mComputedWidth and mComputedHeight
Resolution: FIXED → ---
Removing DOGFOOD/BLOCKER designations and reopening bug per David's comments.
Status: REOPENED → ASSIGNED
Target Milestone: M13 → M14
Moving to M14.
Blocks: 24206
Whiteboard: [PDT-] 1/7: Pending verification from reporter → [PDT-]
Removing PDT annotation now that dogfood nomination has been removed.
Whiteboard: [PDT-]
Moving to M16.
Target Milestone: M14 → 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.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
No longer blocks: 24206
Verifying bug fixed. If reporter does not agree, please reopen.
Status: RESOLVED → VERIFIED
Reopening to mark invalid.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → INVALID
QA Contact: chrisd → amar
 Marking verified. Invalid
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.