Closed Bug 1739048 Opened 9 months ago Closed 9 months ago

Firefox doesn't honor percent height in stack of nested flex containers, inside of `overflow:hidden`

Categories

(Core :: Layout: Flexbox, defect)

defect

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox93 --- wontfix
firefox94 --- wontfix
firefox95 --- wontfix
firefox96 --- fixed

People

(Reporter: dholbert, Assigned: TYLin)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached file testcase 1

STR:

  1. Load attached testcase.

EXPECTED RESULTS:
Lime square.

ACTUAL RESULTS:
Mostly-red square.

mozregression says this regressed with bug 1654044.

We give EXPECTED RESULTS if I remove overflow:hidden from the testcase. Also, Chrome gives EXPECTED RESULTS.

FWIW, this was originally reported via https://webcompat.com/issues/87812 . It causes a minor graphical glitch at the top of https://urologiaenmonterrey.com/servicios -- in affected builds, the "white-to-transparent swoosh" at the top left of the page isn't quite as tall as the photo that it's meant to cover up (the photo with the doctor on the right).

I can fix this bug by forcing FlexItem::NeedsFinalReflow to unconditionally return true.

So, this boils down to a bug in that optimization (skipping the final reflow under certain conditions laid out in that function).

Set release status flags based on info from the regressing bug 1654044

(In reply to Daniel Holbert [:dholbert] from comment #3)

I can fix this bug by forcing FlexItem::NeedsFinalReflow to unconditionally return true.

So, this boils down to a bug in that optimization (skipping the final reflow under certain conditions laid out in that function).

Flex container checks FrameHasRelativeBSizeDependency() to detect whether the final reflow is needed. However, when the flex item is a scroll container, the NS_FRAME_CONTAINS_RELATIVE_BSIZE bit is only set on its inner scrolled frame (-moz-scrolled-content) but not set on the outer scroll HTMLScroll, so FrameHasRelativeBSizeDependency fails to detect the item needs a final reflow.

We walk the parent reflow input chain to set NS_FRAME_CONTAINS_RELATIVE_BSIZE bit in ReflowInput::InitResizeFlags starting from the frame having percentage bsize up to its containing block's reflow input. However, nsIFrame::GetContainingBlock() by default returns the inner scrolled frame for any children of a scroll container (bug 1204044).

Re comment 5:

Flex container checks FrameHasRelativeBSizeDependency() to detect whether the final reflow is needed. However, when the flex item is a scroll container, the NS_FRAME_CONTAINS_RELATIVE_BSIZE bit is only set on its inner scrolled frame (-moz-scrolled-content) but not set on the outer scroll HTMLScroll, so FrameHasRelativeBSizeDependency fails to detect the item needs a final reflow.

To clarify the above claim, the FrameHasRelativeBSizeDependency detection failed in a nested flex containers scenario as demonstrate by the testcase 1 in comment 0.

FlexContainer(div) <-- outer flex 
  FlexContainer(div) <-- inner flex
    HTMLScroll(div) <-- outer scroll frame, has no NS_FRAME_CONTAINS_RELATIVE_BSIZE
      Block(div) <-- inner scrolled frame, has NS_FRAME_CONTAINS_RELATIVE_BSIZE
        Block(div) <-- class="D" that has height: 100% 

When the outer flex container calls NeedsFinalReflow to detect whether the inner flex container needs a final reflow, FrameHasRelativeBSizeDependency can only look one level deep into the child lists, and can only see HTMLScroll. As a result, it failed to detect the inner flex needs a final reflow.

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0bff75093358
Propagate NS_FRAME_CONTAINS_RELATIVE_BSIZE from inner scrolled frame to outer scroll frame. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31601 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
Upstream PR merged by moz-wptsync-bot
QA Whiteboard: [qa-96b-p2]
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.