Closed
Bug 521525
Opened 15 years ago
Closed 15 years ago
Removing height from block doesn't change layout immediately for percent-height kids
Categories
(Core :: Layout, defect, P2)
Core
Layout
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta2-fixed |
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(Keywords: testcase)
Attachments
(4 files)
180 bytes,
text/html
|
Details | |
80 bytes,
text/html
|
Details | |
227 bytes,
text/html
|
Details | |
3.70 KB,
patch
|
dbaron
:
review+
dbaron
:
approval1.9.2+
|
Details | Diff | Splinter Review |
Based on layout/base/crashtests/89101-1.html.
Seems to be quirksmode-only.
Reporter | ||
Comment 1•15 years ago
|
||
![]() |
Assignee | |
Comment 2•15 years ago
|
||
The difference is that in quirks mode mFlags.mVResize ends up false on the reflow state, so we don't reflow the block inside the fieldset, even though it has a percentage height.
The relevant code here seems to be:
} else if (mComputedHeight == NS_AUTOHEIGHT) {
if (eCompatibility_NavQuirks == aPresContext->CompatibilityMode() &&
mCBReflowState) {
mFlags.mVResize = mCBReflowState->mFlags.mVResize;
} else {
mFlags.mVResize = mFlags.mHResize || NS_SUBTREE_DIRTY(frame);
}
In this case, NS_SUBTREE_DIRTY(frame) is true, so in standards mode we get mVResize set.
The frame flags are 0x1021, which does NOT include NS_FRAME_IS_DIRTY for some reason. It really should in this case. Looking into that now.
![]() |
Assignee | |
Comment 3•15 years ago
|
||
And _that_ is a regression from bug 502288. I think I should probably include nsChangeHint_NeedDirtyReflow for height/min-height/max-height changes, though it's still ok to skip for width due to the asymmetry between width and height calculations.
David, does that seem reasonable? I guess the other option is to add the NS_SUBTREE_DIRTY(frame) condition in quirks mode too. Would you prefer that? not sure what the tradeoff there is...
We need to fix this on 1.9.2 as well.
Assignee: nobody → bzbarsky
Blocks: 502288
![]() |
Assignee | |
Comment 4•15 years ago
|
||
![]() |
Assignee | |
Updated•15 years ago
|
Assignee: bzbarsky → nobody
Component: Layout: Form Controls → Layout: Misc Code
Flags: blocking1.9.2?
OS: Mac OS X → All
QA Contact: layout.form-controls → layout.misc-code
Hardware: x86 → All
![]() |
Assignee | |
Updated•15 years ago
|
Summary: Removing height from fieldset doesn't change layout immediately → Removing height from block doesn't change layout immediately for percent-height kids
![]() |
Assignee | |
Comment 5•15 years ago
|
||
Attachment #405914 -
Flags: review?(dbaron)
Comment on attachment 405914 [details] [diff] [review]
Proposed fix
r=dbaron
Attachment #405914 -
Flags: review?(dbaron) → review+
![]() |
Assignee | |
Comment 8•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
![]() |
Assignee | |
Comment 9•15 years ago
|
||
Comment on attachment 405914 [details] [diff] [review]
Proposed fix
Need this on 1.9.2.
Attachment #405914 -
Flags: approval1.9.2?
Comment on attachment 405914 [details] [diff] [review]
Proposed fix
a1.9.2=dbaron
Attachment #405914 -
Flags: approval1.9.2? → approval1.9.2+
Updated•15 years ago
|
Assignee: nobody → bzbarsky
![]() |
Assignee | |
Comment 11•15 years ago
|
||
Note that this patch made some cases that didn't use to reflow correctly (e.g. bug 523096) reflow correctly, presumably because mVResize being true triggers a necessary codepath that they didn't use to trigger for just having a dirty descendant.
![]() |
Assignee | |
Comment 12•15 years ago
|
||
status1.9.2:
--- → final-fixed
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P2
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•