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

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: 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.

Comment 1

[Daniel Holbert [:dholbert]]

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).

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: reference case 1 (just removed 'overflow:hidden' from testcase)

Comment 3

[Daniel Holbert [:dholbert]]

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).

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

(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).

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

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.

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1739048 - Propagate NS_FRAME_CONTAINS_RELATIVE_BSIZE from inner scrolled frame to outer scroll frame. r?dholbert

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bff75093358
Propagate NS_FRAME_CONTAINS_RELATIVE_BSIZE from inner scrolled frame to outer scroll frame. r=dholbert

Comment 9

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31601 for changes under testing/web-platform/tests

Comment 10

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/0bff75093358

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot