Closed Bug 841827 Opened 7 years ago Closed 7 years ago

{inc} abspos flex container w/ "top: 0; bottom: 0" doesn't resize when window is resized

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Attached file testcase 1
STR: Load attached testcase; resize window vertically

EXPECTED RESULTS: The black-bordered flex container should resize vertically w/ the container.

ACTUAL RESULTS: The flex container remains at its initial height (the window-height when the page loaded)
OS: Linux → All
Hardware: x86_64 → All
If I use gdb to forcibly set "shouldReflowChildren" to true, in nsFlexContainerFrame::Reflow,  after vertically resizing the window, then I get EXPECTED RESULTS. (the flex container resizes)

I think we should consider this a case where we've got a relative height (since our height depends on our containing block), so I think we should add this as a condition that makes us add the NS_FRAME_CONTAINS_RELATIVE_HEIGHT bit.

With that bit set, "aReflowState.ShouldReflowAllKids()" will end up returning true (from its mVResize && CONTAINS_RELATIVE_HEIGHT check), and we'll proceed with a full reflow.
Comment on attachment 718808 [details] [diff] [review]
reftest patch

(The reftest patch uses 'hg cp', so it looks like it's deleting a bunch of stuff, but really it's just co-opting the reusable parts.)
Attachment #718808 - Flags: review? → review?(dbaron)
Attachment #718791 - Attachment description: fix v1 → fix v1: treat abspos w/ explicit "top" & "bottom" values as being a relative height
Comment on attachment 718791 [details] [diff] [review]
fix v1: treat abspos w/ explicit "top" & "bottom" values as being a relative height

So the condition for abs-pos should *also* check && eStyleUnit_Auto == stylePos->mHeight.GetUnit() since the height is only a function of 'top' and 'bottom' when the 'height' is 'auto':
http://www.w3.org/TR/CSS21/visudet.html#abs-non-replaced-height

r=dbaron with that fixed


But I'm also a little concerned about whether this is setting the bit on the right set of frames, but this patch is fine.  (Does the parent, given that it's the containing block, also need to have the bit set?  And does the quirks mode percentage height code require setting it in more cases in quirks mode?  I'm comparing to the code that sets this bit in nsHTMLReflowState::InitResizeFlags, which is the primary case for setting this bit.)  Would you mind looking into that?
Attachment #718791 - Flags: review?(dbaron) → review+
Attachment #718808 - Flags: review?(dbaron) → review+
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #6)
> So the condition for abs-pos should *also* check && eStyleUnit_Auto ==
> stylePos->mHeight.GetUnit() since the height is only a function of 'top' and
> 'bottom' when the 'height' is 'auto':
> http://www.w3.org/TR/CSS21/visudet.html#abs-non-replaced-height

Ah, right. Thanks!

> r=dbaron with that fixed
> 
> But I'm also a little concerned about whether this is setting the bit on the
> right set of frames, but this patch is fine.

It sounds like you're OK w/ this patch landing as-is and investigating your question as a followup -- so I pushed this bug's patch (w/ the reftest patch folded in) to inbound:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/fa2ffdf930be
...and I filed bug 851012 on investigating your followup question.
Status: NEW → ASSIGNED
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/fa2ffdf930be
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(In reply to Daniel Holbert [:dholbert] from comment #7)
> It sounds like you're OK w/ this patch landing as-is

(with the height=auto review comment fixed, of course -- I fixed that before landing.)
You need to log in before you can comment on or make changes to this bug.