Closed Bug 282754 Opened 20 years ago Closed 20 years ago

Width:100% broken if parent block element has overflow:hidden

Categories

(Core :: Layout: Positioned, defect)

x86
All
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: alex, Assigned: roc)

References

()

Details

(Keywords: regression, testcase)

Attachments

(7 files)

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/).
Attached file Testcase #1 (deleted) —
Component: Layout: Block and Inline → Layout: R & A Pos
Keywords: regression, testcase
OS: Windows XP → All
QA Contact: layout.block-and-inline → layout.r-and-a-pos
Backing out bug 277420 fixes this
Blocks: 277420
Flags: blocking1.8b2?
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?
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...
Attached patch Patch rev. 1 (deleted) — Splinter Review
Something like this perhaps...
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?
Assignee: nobody → roc
Severity: normal → major
Attached file Testcase from bug 277420 "fix3" (deleted) —
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 on attachment 174827 [details] [diff] [review] Patch rev. 1 Robert, do you recall why you had to change the code that effectively did this?
Attachment #174827 - Flags: review?(roc)
Attached file abs-pos columns testcase (deleted) —
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.
Robert, could you add that testcase to the regression tests? And could you land the regression test changes from "fix3" in bug 277420 too?
okay, I checked in the testcases.
*** Bug 282100 has been marked as a duplicate of this bug. ***
Comment on attachment 174827 [details] [diff] [review] Patch rev. 1 would like a new patch here
Attachment #174827 - Flags: review?(roc) → review-
Attached file testcase with all types of overflow (deleted) —
Attached file Testcase (deleted) —
Another testcase; I believe it's the same bug. X and Z are position:absolute right:0.
Blocks: 284620
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.
Status: NEW → ASSIGNED
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?
(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.
Attached patch stopgap fix (deleted) — Splinter Review
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.
Attachment #176684 - Flags: superreview?(dbaron)
Attachment #176684 - Flags: review?(dbaron)
Blocks: 285322
Flags: blocking-aviary1.1?
*** Bug 285491 has been marked as a duplicate of this bug. ***
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
Attachment #176684 - Flags: superreview?(dbaron)
Attachment #176684 - Flags: superreview+
Attachment #176684 - Flags: review?(dbaron)
Attachment #176684 - Flags: review+
regression tested and checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Blocks: 287127
roc, check in the testcases in this bug into the regression tests?
No longer blocks: 284620
*** Bug 284620 has been marked as a duplicate of this bug. ***
*** Bug 285322 has been marked as a duplicate of this bug. ***
Depends on: 287352
This fix appears to cause the regression of bug 287352
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?
*** Bug 286902 has been marked as a duplicate of this bug. ***
Depends on: 294934
Depends on: 307076
Depends on: 307158
(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?
No, that's really hard.
(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?
It's definitely not this bug.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: