View the attached testcase.
The inner content stops at its bottom border, even though there remains
some inner content margin and container padding to display.
The scrollbars should allow access to all the content, even the whitespace.
Currently, we are probably deciding that margins are not content and so we only
let the user scroll to the lowest border, which means that there is no way of
giving a margin to overflowed scrolled content.
From the spec:
# 10.6.3 Block-level, non-replaced elements in normal flow, and floating,
# non-replaced elements
# If [a block box] has block-level children, the height is the distance
# between the top border-edge of the topmost block-level child box and the
# bottom border-edge of the bottommost block-level child box. However, if the
# element has a non-zero top padding and/or top border, then the content
# starts at the top margin edge of the topmost child. Similarly, if the
# element has a non-zero bottom padding and/or bottom border, then the
# content ends at the bottom margin edge of the bottommost child.
Windows 2000 Commercial Build 18.104.22.1680080404.
Originally seen on a recent Mac build.
This is a regression. See bug 2751 and bug 13497.
Created attachment 12403 [details]
Self explanatory testcase.
This looks like a layout issues, not style. Reassigning to buster and marking
future since he will probably not get a chance to look at it any time soon due
to his general doomage factor being high.
Also, marking beta3- since there is no content lost and nothing crashes
(rigorous criterion, eh?)
Seems we are drawing the scrollbars in a strange place too, which is fun. This
is particularly visible with the bottom scrollbar on
...(although that page has other errors).
Build reassigning Buster's bugs to Marc.
I've noticed this before.
*** Bug 225396 has been marked as a duplicate of this bug. ***
So the problem is that mOverflowArea doesn't include margins (nor should it, per
documentation in nsHTMLReflowMetrics.h), right?
I'm not sure about how collapsing margins are supposed to work here.
One thing I'm sure of is that the padding on the scrolled DIV is in the scrolled
area at the top, left and right but not on the bottom. That is a bug for sure.
Exactly which case are you not sure about? I currently understand collapsing
margins pretty well and can probably explain any case you put forward.
In the testcase, does the 2em margin-bottom on the inner DIV collapse with a
margin on a subsequent DIV? If so, should that margin area be considered to be
part of the inner DIV that overflows the outer DIV?
From CSS2.1, section 8.3.1:
# Vertical margins of elements with 'overflow' other than 'visible' do not
# collapse with their in-flow children.
*** Bug 251553 has been marked as a duplicate of this bug. ***
*** Bug 259008 has been marked as a duplicate of this bug. ***
*** Bug 269322 has been marked as a duplicate of this bug. ***
Would it be ok to change "inner padding" to "padding-bottom" in the summary to
help duplicate searches? It seems to me that only padding-bottom is involved.
My 2 cents.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050428
Since the landing of bug 240276 there is now the expected gap between the bottom
of the div and the bottom of the containing scrollbox. However, the gap between
the right of the div and the containing scrollbox now appears to be twice a big
as it was previously.
Created attachment 182143 [details] [diff] [review]
Yeah, the padding in this testcase is still wrong.
The change to have the reflow state set zero padding for the scrollframe is
wrong. That confuses everything because mComputedWidth/Height on the
scrollframe ends up including the element's padding, but the nsGfxScrollFrame
code keeps assuming it excludes the padding.
This patch backs out that change and modifies nsHTMLScrollFrame::Reflow and
friends to explicitly avoid using the padding stored in the reflow state.
This is just fixing the inner padding aspects of this bug, not the content
margins aspects, right?
Er, actually, I could imagine it fixing both for the vertical case, but not for
horizontal, where that's a lot more work. Perhaps it should be filed as a
Created attachment 182358 [details]
screenshot of testcase
It all looks fine to me, 4em of whitespace on each side of the content's
border-box. If I remove all the padding, I still see 2em of whitespace on each
side. What problem are you expecting to see? Margin on the inner DIV gets
included in the overflow area of the outer DIV (the scrolled frame), and margin
on the outer DIV isn't a problem of course.
I see how it can fix the margins problem for a subset of the vertical case, but
not for the general case. Consider:
<div style="overflow: auto; height: ...; width: ...">
<div style="width: 130%; margin-right: 10%">
<div style="overflow: auto; height: 100px; width: ...">
<div style="height: 0">
<div style="height: 200px; margin-bottom: 50px">
I think this bug is suggesting that we should scroll to show those margins. I
don't think this patch does that, since it's a lot more work. I'm not sure if
the spec requires it or what other browsers do.
I think the spec at least intends to require it; see comment 0. I haven't
checked to see if we changed the spec in the last 5 years though.
Comment on attachment 182143 [details] [diff] [review]
fixes a layout regression I just introduced
Comment on attachment 182143 [details] [diff] [review]
checked in. I'll leave this open for the margin issue.
I checked in the wrong patch. I'll check in the right patch tomorrow.
reopening because the content margin issue is still there.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050603
The testcase now WFM.
The testcase WORKSFORME with Mozilla 1.8b2 build 2005070105 and Firefox Deer
Park alpha 1 build 20050706. Is there still an issue?
See comment 24.
So I think the content margin part of this is invalid, at least for the horizontal case (and preferably for both due to symmetry), because the CSS spec says that margin-right in an overconstrained case is ignored and becomes negative.
*** Bug 322881 has been marked as a duplicate of this bug. ***
Note that the right padding also is missing for wide content which invokes the horizontal scroll bar See:
... and I think bug 458296 is fixing the vertical margin case.
So maybe this is fixed given comment 31, comment 36, and this?
Er, I should say *will be* fixed.
Actually, it isn't really, since that only considers the margins on children and not margins of their descendants.
What are you referring to in comment #41? Bug 458296 ensures that the carried-out bottom margin of a scrolled block is added to the scrollable area. That includes the carried-out margins of block descendants if they collapse through to the bottom of the scrolled content. So given
the height of the scrollable area will be 70px. Are you saying it should be 120px?
Yes, although the case I was thinking of was actually:
<div style="height: 10px">
<div style="margin-bottom: 10px">
lots of contents
That said, I suspect other browsers don't do that, and as far as I can tell the spec doesn't actually define what to do here, so perhaps our behavior with bug 458296 is good enough.
The (In reply to comment #43)
> <div style="height: 10px">
> <div style="margin-bottom: 10px">
> lots of contents
The patch actually handles this case. Reflow of the outer div sets bottomEdgeOfChildren to include the outgoing bottom margin (because of the NS_UNCONSTRAINEDSIZE != aReflowState.ComputedHeight() test).
Sorry, I meant that there was a
*outside* what I wrote in comment 43 (not that the overflow:scroll was on the outer div there).
That still works. nsBlockFrame::ComputeCombinedArea adjusts the overflow area to include bottomEdgeOfChildren for all blocks, so the height:10px div's overflow area includes the child's bottom margin, and the overflow area propagates up to the scrolled parent.
Ah, right. So I guess this will be fixed, except for a few very obscure direction-swapping cases, e.g.:
<div style="direction:ltr; overflow:scroll">
<div style="position: relative; right: 50px; margin-right: 50px">
that aren't covered by comment 36.
Created attachment 8449739 [details]
Just came across this issue in production (at least I *think* it's the same one). I'm attaching a minimal html file that reproduces the problem, along with another file that can be used as a reference for what should(?) happen.
Created attachment 8449740 [details]