KawaiiBASIC / classilla

Automatically exported from code.google.com/p/classilla
0 stars 0 forks source link

CSS height % and min-height problems [68kmla.net] #48

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This is a common problem, plus it's on a site that a lot of Classilla
evaluators also read, so it should be fixed. As shown on issue 27, it is
distinct from the floodgap shifted div problem. It is fixed in Moz 1.6, so
it likely predates 300030 (thank goodness).

Original issue reported on code.google.com by classi...@floodgap.com on 20 Aug 2009 at 4:43

GoogleCodeExporter commented 9 years ago
Analysis of a minimal case shows that it almost entirely renders correctly if 
the CSS

li.header dl.icon {
        min-height: 0;
}

has min-height removed.

Original comment by classi...@floodgap.com on 20 Aug 2009 at 4:44

GoogleCodeExporter commented 9 years ago
This is probably related, although it is probably not this exact problem. It is
trivial to apply, so we should:
https://bugzilla.mozilla.org/show_bug.cgi?id=220266

Original comment by classi...@floodgap.com on 20 Aug 2009 at 4:56

GoogleCodeExporter commented 9 years ago
Other zero height issues:
https://bugzilla.mozilla.org/show_bug.cgi?id=186516

Original comment by classi...@floodgap.com on 20 Aug 2009 at 5:01

GoogleCodeExporter commented 9 years ago
If we make the min-height:1; it also works. Perhaps this could be a kludge if I 
can't
find another way around it. It's sort of a baffling case anyway.

Original comment by classi...@floodgap.com on 20 Aug 2009 at 5:04

GoogleCodeExporter commented 9 years ago
The linklist is repaired by removing overflow:hidden from 

* html .clearfix, * html .navbar, ul.linklist {
        height: 4%;
        overflow: hidden;
}

or by setting a proper px height, so it's probably not the overflow that is the
issue. This is also unrelated but we should try to fix this too, it is 
undoubtedly a
problem on other sites.

Original comment by classi...@floodgap.com on 20 Aug 2009 at 5:16

GoogleCodeExporter commented 9 years ago
not 219693

Original comment by classi...@floodgap.com on 20 Aug 2009 at 5:22

GoogleCodeExporter commented 9 years ago
216303 is a candidate also. we'll start with these.

https://bugzilla.mozilla.org/show_bug.cgi?id=182748 may also be useful.

so far nothing matches the min-height problem. in
layout/html/base/src/nsHTMLReflowState.cpp
we might adjust ::ComputeMinMaxValues. We probably want to mask
mStylePosition->mMinHeight not the computed value such that if it evaluates to 
zero,
a one is placed, and only if explicit units are specified.

This looks like a good place:

  nsStyleUnit minHeightUnit = mStylePosition->mMinHeight.GetUnit();
  if (eStyleUnit_Inherit == minHeightUnit) {
    mComputedMinHeight = aContainingBlockRS->mComputedMinHeight;
  } else {
    // Check for percentage based values and a containing block height that
    // depends on the content height. Treat them like 'auto'
    if ((NS_AUTOHEIGHT == aContainingBlockHeight) &&
        (eStyleUnit_Percent == minHeightUnit)) {
      mComputedMinHeight = 0;
    } else {
      ComputeVerticalValue(aContainingBlockHeight, minHeightUnit,
                           mStylePosition->mMinHeight, mComputedMinHeight); // <<< HERE
    }
  }

Original comment by classi...@floodgap.com on 20 Aug 2009 at 5:50

GoogleCodeExporter commented 9 years ago
Might also have to look at
content/html/style/src/nsComputedDOMStyle.cpp
under ::GetMinHeight

Original comment by classi...@floodgap.com on 20 Aug 2009 at 6:00

GoogleCodeExporter commented 9 years ago
changing issue name while I mull this over

Original comment by classi...@floodgap.com on 23 Aug 2009 at 6:38

GoogleCodeExporter commented 9 years ago
Fix-up shim placed in nsCSSDeclaration.cpp which prevents assertion of any 
min-height
rule of zero. This yielded the following interesting effects:

- The site now mostly renders and is useable, even though it doesn't render 
right.
- The cells are fixed size and do not stretch to the size of their contents.
- When the contents of a cell do overflow, the overflow is hidden, *and* 
subsequent
rows are shifted right.
- The cells appear to be display:block in the CSS, not a true table.

Therefore, there are two separate problems:

- Cell height is not being computed correctly.
- As soon as cell height overflows a minimum computed height (either hard coded 
or
computed by layout/content???), the display breaks down. (Thus the min-height 
fix-up
is a bandaid on this second problem, since the cell will overflow immediately 
if a
min-height:0 is specified, and will overflow *later* if it isn't, but later 
enough
that the entire display doesn't blow chunks).

The first issue may be very difficult to repair in this release; we're dealing 
with a
lot of miscomputed height issues even in 9.0.4-internal. However, the second one
shouldn't be as difficult. I may pull up key parts of nsTable*.cpp and see how 
much
good that does even though these are CSS blocks. Even without this, the fixup 
is ugly
but shippable.

Original comment by classi...@floodgap.com on 26 Sep 2009 at 8:01

GoogleCodeExporter commented 9 years ago
More and more interesting. Test cases in DOM Inspector show that the shift-over 
is
actually very logical when you realize what the view manager is actually doing. 
It
sees the cell after the overflowed cell as a floating cell to be floated NEXT 
to it
(you can see this by highlighting the relevant cells in DOM Inspector). If you 
add
enough rows, the cell will then flow back to the left and resume correct layout 
until
there is another height overflow. This is also why the bottom border seems to 
thicken
up; it, too, depends on the height of the overflowed right-shifted cell.

The faulty logic is then not the cell-shift; that is a logical consequence. The
problem is that the height of the row fails to increase to its largest size 
despite
overflow:visible. So, again, we "simply" need to solve why height of the cell 
is not
being propagated to the entire block. 

Original comment by classi...@floodgap.com on 29 Sep 2009 at 5:04

GoogleCodeExporter commented 9 years ago
Fixed by M69355/mm and friends, along with a crapload of other layout fixes. We 
now
render 68kmla correctly!

Original comment by classi...@floodgap.com on 15 Oct 2009 at 4:41

GoogleCodeExporter commented 9 years ago
And we had to back that out (see issue 66). Min-height fixup is shipping in 
core,
with the fixup renderer option, and this as stated above makes the site usable, 
even
though it still doesn't render right.

Since we know what the correct fix is, and we can't land that yet, and when we 
do
land it this bug will be fixed, I'll leave this Verified.

Original comment by classi...@floodgap.com on 20 Oct 2009 at 4:38