Last Comment Bug 282754 - Width:100% broken if parent block element has overflow:hidden
: Width:100% broken if parent block element has overflow:hidden
Status: RESOLVED FIXED
: regression, testcase
Product: Core
Classification: Components
Component: Layout: R & A Pos (show other bugs)
: Trunk
: x86 All
: -- major with 3 votes (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
: Jet Villegas (:jet)
Mentors:
http://blog.rd2inc.com/testing/overfl...
: 282100 284620 285322 285491 286902 (view as bug list)
Depends on: 287352 294934 307076 307158
Blocks: 277420 285322 287127
  Show dependency treegraph
 
Reported: 2005-02-18 13:00 PST by Ashley Bischoff (blog at handcoding.com)
Modified: 2006-09-24 15:30 PDT (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase #1 (620 bytes, text/html)
2005-02-19 17:05 PST, Mats Palmgren (:mats)
no flags Details
Patch rev. 1 (1.37 KB, patch)
2005-02-19 19:15 PST, Mats Palmgren (:mats)
roc: review-
Details | Diff | Splinter Review
Testcase from bug 277420 "fix3" (1.71 KB, text/html)
2005-02-20 17:21 PST, Mats Palmgren (:mats)
no flags Details
abs-pos columns testcase (451 bytes, text/html)
2005-02-20 19:55 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
no flags Details
testcase with all types of overflow (2.18 KB, text/html)
2005-02-27 20:02 PST, Mathieu Pellerin
no flags Details
Testcase (888 bytes, text/html)
2005-02-28 12:17 PST, Phil Endecott
no flags Details
stopgap fix (8.86 KB, patch)
2005-03-07 20:05 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review

Description Ashley Bischoff (blog at handcoding.com) 2005-02-18 13:00:06 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050218 Firefox/1.0+ (BlueFyre)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050218 Firefox/1.0+ (BlueFyre)

While width:100% on the child element of a block-level element normally works as
expected, if the parent element has overflow:hidden, then the child incorrectly
calculates its width.

Reproducible: Always

Steps to Reproduce:
1. Load the testcase at the URL above (or here:
http://blog.rd2inc.com/testing/overflow-hidden/overflow-hidden-testcase.html)
2. If you view the source, you can see  that both instances have a span tag
which is set to width:100% and height:100%.

Actual Results:  
When, overflow:hidden is applied to the parent element, the child is unable to
calculate the full width of the parent.

Expected Results:  
The child element should expand to the width of its parent with width:100% is
applied.

I wasn't quite sure which component to set for this bug, but feel free to
transfer it to another component if another would be more appropriate.

And, Firefox didn't used to have this bug, but it crept in somewhere during the
last few builds.

I ran into this when implementing the Gilder Image Replacement technique
(http://blog.tom.me.uk/2003/08/07/) and I borrwed the sample image from Dave
Shea's image replacement comparison
(http://www.mezzoblue.com/tests/revised-image-replacement/).
Comment 1 Mats Palmgren (:mats) 2005-02-19 17:05:45 PST
Created attachment 174820 [details]
Testcase #1
Comment 3 Mats Palmgren (:mats) 2005-02-19 17:37:05 PST
Backing out bug 277420 fixes this
Comment 4 Mats Palmgren (:mats) 2005-02-19 18:55:32 PST
Restoring the original logic for when to return -1,-1 in
CalculateContainingBlockSizeForAbsolutes() fixes the problem.

Before bug 277420:
Area(p)(1)@0x8702a1c   => (mPosition=0) -1,-1
Area(p)(3)@0x87039a4   => (mPosition=1) 4606,350

After bug 277420:
Area(p)(1)@0x8702b34   => (mPosition=0) 434,350
Area(p)(3)@0x8703abc   => (mPosition=1) 4606,350

#define NS_STYLE_POSITION_STATIC                0
#define NS_STYLE_POSITION_RELATIVE              1

Hmm, why is the first <p> STATIC?
Is this some inner scroll frame or something?
Comment 5 Mats Palmgren (:mats) 2005-02-19 19:03:30 PST
FWIW, this change in "ua.css" makes it RELATIVE:
 *|*::-moz-scrolled-content {
...
-  position: static !important;
+  position: inherit !important;
...
 }

So it seems we need to add a check for STATIC as well as the initial block...
Comment 6 Mats Palmgren (:mats) 2005-02-19 19:15:40 PST
Created attachment 174827 [details] [diff] [review]
Patch rev. 1

Something like this perhaps...
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2005-02-20 15:01:14 PST
Hmmm...  So the code in nsCSSFrameConstructor::GetAbsoluteContainingBlock is
also wrong, actually (it doesn't return the same thing that we used as the
containing block in ConstructBlock when columns are used).

The patch here looks like it effectively backs out the change to
CalculateContainingBlock that was made.  Wouldn't that cause the problems
described in bug 277420 comment 10?
Comment 8 Mats Palmgren (:mats) 2005-02-20 17:21:47 PST
Created attachment 174933 [details]
Testcase from bug 277420 "fix3"

This is the testcase in the "fix3" patch from bug 277420 comment 10.
I dumped the frame trees before and after the patch here and the trees are
identical.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2005-02-20 18:46:12 PST
Comment on attachment 174827 [details] [diff] [review]
Patch rev. 1

Robert, do you recall why you had to change the code that effectively did this?
Comment 10 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-02-20 19:55:46 PST
Created attachment 174945 [details]
abs-pos columns testcase

This testcase doesn't work with Mats' patch, and there are assertion failures:

###!!! ASSERTION: containing block height must be constrained:
'containingBlockHeight != NS_AUTOHEIGHT', file
../../../layout/generic/nsHTMLReflowState.cpp, line 982
Break: at file ../../../layout/generic/nsHTMLReflowState.cpp, line 982

We end up positioning the BL at the bottom of the universe because the column
block is being reflowed with unconstrained height.

In this bug's testcase I'm not really sure from the spec whether we're supposed
to be honouring the "internal" or "external" dimensions of the element. I guess
external. I suppose then that me setting the internal column block as the
abs-pos container is technically incorrect. Hmm... Then we should fix it so
ConstructBlock always sets the outer block as containing block.
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2005-02-20 20:12:43 PST
Robert, could you add that testcase to the regression tests?  And could you land
the regression test changes from "fix3" in bug 277420 too?
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-02-20 20:46:41 PST
okay, I checked in the testcases.
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2005-02-22 18:37:48 PST
*** Bug 282100 has been marked as a duplicate of this bug. ***
Comment 14 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-02-24 19:56:45 PST
Comment on attachment 174827 [details] [diff] [review]
Patch rev. 1

would like a new patch here
Comment 15 Mathieu Pellerin 2005-02-27 20:02:18 PST
Created attachment 175785 [details]
testcase with all types of overflow
Comment 16 Phil Endecott 2005-02-28 12:17:22 PST
Created attachment 175841 [details]
Testcase

Another testcase; I believe it's the same bug.	X and Z are position:absolute
right:0.
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-07 19:32:04 PST
We have a dilemma. For blocks wrapped in some other type of frame, it's the
dimension of the outer (primary) frame that should determine the absolute
positioning. But sometimes we don't know the dimensions of the outer frame until
we've finished reflowing the block and then the outer frame. So what we really
need to do is to reflow the block and then the outer frame and then go back and
reflow the absolutes. But that's really ugly.
Comment 18 David Baron :dbaron: ⌚️UTC-8 2005-03-07 19:44:46 PST
Not necessarily.  I've been thinking that perhaps the best solution to the
various incremental reflow issues we have with absolutely positioned elements is
to call their Reflow methods in a separate pass after the rest of Reflow.  The
one problem is that it affects overflow areas (and thus scrollbar presence too).

Were you referring to intrinsic width issues or scrollbar presence/absence issues?
Comment 19 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-07 19:59:31 PST
(In reply to comment #18)
> Not necessarily.  I've been thinking that perhaps the best solution to the
> various incremental reflow issues we have with absolutely positioned elements
> is to call their Reflow methods in a separate pass after the rest of Reflow.
> The one problem is that it affects overflow areas (and thus scrollbar presence
> too).

Right, so it could change the intrinsic dimensions of scrollframes.

The actual right solution to this particular problem, and some other problems,
IMHO, is to make "absolute container" be applicable to any type of frame as some
sort of frame property. We definitely need this for all the non-block frames,
that don't wrap blocks, that can be relatively positioned --- especially tables.
But it would also avoid the problems in comment 17, because we'd always reflow
the absolute frames right after we'd computed the final size of the container.
Comment 20 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-07 20:05:02 PST
Created attachment 176684 [details] [diff] [review]
stopgap fix

Short of a comprehensive fix like that, I propose this fix.

The nsCSSFrameConstructor part just makes GetAbsoluteContainingBlock consistent
with ConstructBlock and simplifies/generalizes the code a bit with the help of
GetContentInsertionFrame. It's really unrelated to the actual fix.

The actual fix in CalculateContainingBlockSizeForAbsolutes finds the reflow
state for the outermost frame for this content. If that reflow state has a
computed width or height, then we use that value for the container dimension.
This means that at least elements with non-auto width and height will do the
right thing. Otherwise we do the best we can, which is what we're currently
doing. At least all the testcases in this bug work.
Comment 21 Andrew Schultz 2005-03-12 22:24:44 PST
*** Bug 285491 has been marked as a duplicate of this bug. ***
Comment 22 David Baron :dbaron: ⌚️UTC-8 2005-03-18 15:16:38 PST
Comment on attachment 176684 [details] [diff] [review]
stopgap fix

Make the code in nsBlockFrame a little more careful about what happens if one
of the parentReflowState pointers is null (consider reflow roots or box/block
interfaces in addition to just the root of the frame tree), and r+sr=dbaron
Comment 23 Robert O'Callahan (:roc) (email my personal email if necessary) 2005-03-21 13:44:06 PST
regression tested and checked in.
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2005-03-22 12:51:14 PST
roc, check in the testcases in this bug into the regression tests?
Comment 25 Bob Clary [:bc:] 2005-03-22 13:09:23 PST
*** Bug 284620 has been marked as a duplicate of this bug. ***
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2005-03-22 19:32:08 PST
*** Bug 285322 has been marked as a duplicate of this bug. ***
Comment 27 Peter van der Woude [:Peter6] 2005-03-23 06:04:51 PST
This fix appears to cause the regression of bug 287352
Comment 28 Boris Zbarsky [:bz] (still a bit busy) 2005-04-12 20:19:06 PDT
*** Bug 286902 has been marked as a duplicate of this bug. ***
Comment 29 Phil Endecott 2006-02-12 05:50:36 PST
(In reply to comment #20)
> all the testcases in this bug work.

I'm testing with today's nightly build, which I think must have this fix in it.  Looking at the testcase that I submitted (https://bugzilla.mozilla.org/attachment.cgi?id=175841&action=view) it seems that it's not entirely fixed; in the third rectangle, the letter Z is not visible.  This is a horizontal vs. vertical issue.  Can the fix be tweaked slightly to apply to vertical dimensions too?


Comment 30 Robert O'Callahan (:roc) (email my personal email if necessary) 2006-02-12 15:20:42 PST
No, that's really hard.
Comment 31 Phil Endecott 2006-09-24 14:32:18 PDT
(In reply to comment #30)
> No, that's really hard.

It might be really hard, but I think it's still a bug.  Is it still *this* bug (reopen?), or a different existing bug, or a new bug?
Comment 32 Boris Zbarsky [:bz] (still a bit busy) 2006-09-24 14:43:44 PDT
It's definitely not this bug.
Comment 33 Mats Palmgren (:mats) 2006-09-24 15:30:17 PDT
... it's bug 211562.

Note You need to log in before you can comment on or make changes to this bug.