1564261 - incremental layout issue with nested column-oriented flex containers and percent-height descendant (due to inconsistent judgements about whether inner flex container's height is definite)

Status: RESOLVED DUPLICATE of bug 1092007

(Core :: Layout: Flexbox, defect, P3)

Description

[erev0s]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.103 Safari/537.36 OPR/60.0.3255.170

Steps to reproduce:

You can see a codepen example here -> https://codepen.io/anon/pen/agRwpy
and a post in stackoverflow here as well -> https://stackoverflow.com/questions/56934106/after-selector-messes-other-elements

If you hover the navbar in the codepen you will see how the text below changes its position. This does not happen when i remove the ::after selector from the css of the nav-item and this only happens in Firefox.

The ::after selector seems to have a problem when inside a flexbox.

Actual results:

When you hover over the navbar the text below changes position.
https://codepen.io/anon/pen/agRwpy

Expected results:

When i hover the navbar the text below should not move at all.

Comment 1

[Liviu Seplecan]

Hi @erev0s, checked the issue on several Systems: Windows 10, Mac OS X, Ubuntu 18.04 - as well on different FF versions: Nihgtly 70.0a1, beta 68.0b4, release 67.0.4
=> the issue occurs only on Windows 10 OS system. Something to mention is the fact that the FF window should be maximized(the icon next to the "X"). The action will be hover on one element from nav bar -> refresh the page then hove-over again.
I will set a component to it, if isn't the right one please fell free to change it.
Thanks.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Yeah, this is a layout bug, I can repro on Linux as well.

However as-is this is pretty hard to debug, a more reduced test-case would be pretty awesome. I'll give a shot at constructing one later.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case

I think this is as reduced as it gets.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Mozregression says: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6ef8cd81d40542b7319127fdb8be840ab16c4cd4&tochange=7845d51b6f3cd691ec3ec97061021f18cda74c0c

That is, bug 1490890.

This one is weird, I guess there are two different measuring reflows there, and we're incorrectly using the cache from the last reflow on the first reflow of the next reflow. Or something.

Daniel, do you have cycles to investigate? If you want me to take this I'm happy to, otherwise, since I added the cache in the first place. just ni? me back :)

Comment 5

[Daniel Holbert [:dholbert]]

Yup, I'll hopefully have some cycles to take a look early next week. Leaving needinfo open. Thanks!

Comment 6

[Daniel Holbert [:dholbert]]

So the relevant question here is whether the inner flex container's height is considered "definite" for the purposes of resolving the flex-basis:50% on its second child.

In the initial reflow (and reflows that are generated from e.g. resizing the width of the Firefox window), we start reflowing the inner flex container via a call to MeasureAscentAndBSizeForFlexItem() for the outer flex container measuring its child (the inner flex container). At that point, we don't know the inner flex container's BSize yet, because we've only just started reflowing it; so it makes sense for its BSize to be unknown/unconstrained in this codepath. Consequently, we resolve the 50% flex-basis to auto i.e. 0, since it doesn't have any more concrete basis available yet (and it doesn't have any content, so its auto-height is 0). And then, when we're unwind back up to finishing off the outer flex container's reflow, we skip its sole flex item's final reflow (i.e. we skip the final reflow for the inner flex container), because its size hasn't changed size since the earlier "measuring reflow".

However! In subsequent incremental reflows that are generated e.g. by the text-hover, we see that we have a cached BSize measurement for the inner flex container (as a flex item) already, so we take the early-return path from MeasureAscentAndBSizeForFlexItem. This means we don't skip its final reflow, which means we reflow it with a known-in-advance and hence "definite" BSize -- and during that reflow, its percent-flex-basis flex item does have something to resolve its flex-basis against. So it resolves to a nonzero height, which is a layout change.

So, what's the right thing to do here? Really, the inner flex container's main-size should never be considered definite (i.e. we should never use it for resolving the percent height on its child), because it doesn't satisfy the conditions at https://drafts.csswg.org/css-flexbox/#definite-sizes. So we should be adding some special-case code to make its height treated as indefinite even though we have a resolved value for it.

So, this is kind of a version of bug 1092007, though it's a version that manifests in a funny way due to the interaction with this cached-measurement optimization.

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to erev0s from comment #0)

You can see a codepen example here -> https://codepen.io/anon/pen/agRwpy
[...]
If you hover the navbar in the codepen you will see how the text below changes its position.

I can still reproduce this using Firefox 68.0.2, and I can confirm that it doesn't happen in latest Nightly, which has bug 1092007's patch.

Same goes for the attached reduced testcase -- in Firefox 68.0.2, some of the purple changes to red when I hover the text, but that doesn't happen in Nightly.

So, this was effectively a special case of bug 1092007. --> Marking as a duplicate of that bug.