Flexbox align-items causes unstable layout
Categories
(Core :: Layout: Flexbox, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr78 | --- | unaffected |
| firefox-esr91 | --- | wontfix |
| firefox91 | --- | wontfix |
| firefox92 | --- | wontfix |
| firefox93 | --- | fixed |
People
(Reporter: hk.henrik, Assigned: TYLin)
References
(Regression)
Details
(Keywords: regression)
Attachments
(5 files)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:85.0) Gecko/20100101 Firefox/85.0
Steps to reproduce:
See the following fiddle: https://jsfiddle.net/6k79q42s/2/
I have four adjacent divs, each taking up 50% with display set to inline-flex causing two rows of divs.
Each div is identical and contains a div with "display: flex; align-items: center;" + a input.
Actual results:
The initial render renders one of the outer divs mis-aligned, like this: https://i.imgur.com/ZFJL7lh.png.
Subsequent renders places all the divs aligend: https://i.imgur.com/pqs3PRS.png, but I don't think that is completely deterministic. In a real world application I'm working on a similar setup jumps around seemingly randomly when other updates to the DOM are made.
Expected results:
All 4 elements should be aligned next to each other: https://i.imgur.com/pqs3PRS. This works as expected in Chrome-based browsers.
|
||
Comment 1•3 years ago
|
||
I can repro, thanks for the test-case!
|
|
||
Comment 2•3 years ago
|
||
|
||
Updated•3 years ago
|
|
|
||
Comment 4•3 years ago
|
||
I suspect this is an issue for inputs because line-height: -moz-block-height; (used in ::text-control-editing-root) might not set the bsize-dependent flag. But I could be wrong of course, this is just a hunch.
|
Reporter | |
Comment 5•3 years ago
|
||
Thanks for looking into it! That theory sounded plausible, so I tried explicitly setting all line heights it in the app I'm writing, but sadly to no effect.
Again, this is reproducible in a fiddle. All inputs were replaced with divs, and all line-heights are explicitly set: https://jsfiddle.net/o3hjmbks/
|
|
||
Comment 6•3 years ago
•
|
||
Yeah, that line-height is set on an internal pseudo-element so you can't override it, see this line of code
(edit: can't read)
|
||
Comment 7•3 years ago
|
||
Marking this as S2, given that it's a relatively recent regression and seems like it could be quite disruptive for real sites.
Possibly the same underlying issue as bug 1672640?
|
||
Updated•3 years ago
|
|
||
Comment 8•3 years ago
|
||
For the record, here's a standalone copy of reporter's testcase from comment 5 (with no inputs).
|
|
||
Comment 9•3 years ago
|
||
Here's a further-reduced version.
Looks like we're inconsistent about how we're calculating the baseline of the inline-flex element. When you hit "refresh layout", it looks like we mistakenly give it the baseline that the letter "D" would have had if it weren't end-aligned.
|
Assignee | |
Comment 10•3 years ago
|
||
Looks like we cache a mAscent in CachedBAxisMeasurement, but we don't cache/update the ascent after the item's final reflow, and the ascent value in final reflow can be different from the value in content block-size measuring reflow. So next time we reused the values in the cache, we use the wrong ascent value.
I haven't thought much on the solution. Maybe we can move mAscent from CachedBAxisMeasurement to CachedFlexItemData, and update it after both reflow?
|
|
||
Comment 11•3 years ago
|
||
I think our current optimizations effectively assume that a an auto-BSize flex item's ascent will be the same, regardless of whether it actually takes on that "auto" BSize vs. if it's given some other align-self:stretch-imposed BSize.
In other words: we're assuming that stretching the height won't impact its baseline.
This assumption does hold, if the item is a simple block with a paragraph of text, since additional height will go at the bottom and wont impact the ascent. But if the item does any sort of internal vertical alignment of its content (and that content gives it its ascent), then this assumption clearly doesn't hold.
(I don't think we intended to encode this assumption in, but it kinda falls out of the way our flex-item reflow-skipping mechanisms end up combining, I think.)
|
|
||
Comment 12•3 years ago
|
||
Here's a diagnostic patch that effectively disables one of our caching mechanisms. I've confirmed that this fixes all three attached testcases. It also fixes the WPT css/css-flexbox/dynamic-baseline-change.html (which is currently annotated as failing).
(This isn't an actual fix, since we'd like to keep this caching, obviously; it just needs a bit more subtlety.)
|
|
Assignee | |
Comment 13•3 years ago
|
||
CachedBAxisMeasurement::mAscent caches the ascent of a flex item after
the measuring reflow, but the ascent may change after the final reflow
if the item is stretched and does some vertical alignment internally.
However, we don't cache the new ascent. Therefore, when we reflow the
item incrementally, if the CachedBAxisMeasurement::Key is valid, we just
skip the measuring reflow, and retrieve the wrong ascent from the cache.
Instead of fixing this bug by updating the cached ascent or rejecting
the ascent cache for a stretching flex item in block axis, this patch
removes the cache and sets ReflowOutput's BlockStartAscent() to the flex
item after the item's measuring reflow. (We've done the same after the
item's final reflow.) If the ascent is ReflowOutput::ASK_FOR_BASELINE,
we resolve in FlexItem::ResolvedAscent() anyway.
|
||
Updated•3 years ago
|
|
||
Comment 14•3 years ago
|
||
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/20e053eb704a Don't cache ascent in CachedBAxisMeasurement. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29902 for changes under testing/web-platform/tests
|
||
Comment 16•3 years ago
|
||
Backed out for causing reftest failures in flexbox-align-self-baseline-horiz
Backout link: https://hg.mozilla.org/integration/autoland/rev/eba83205abe13ff5abac9b31661b089464aeebcd
Upstream PR was closed without merging
|
|
Assignee | |
Comment 18•3 years ago
|
||
flexbox-align-self-baseline-horiz-3.xhtml failed after my patch removing the ascent cache in flexbox. The reason is because we failed to get the baseline for <button> in incremental reflow. This is only my gut feeling by looking at the frame tree because HTMLButtonControl has block frame as is inner frame, and block frame is reluctant to reflow its lines if they are not dirty.
HTMLButtonControl(button)(3)@7fbcda233f28 parent=7fbcda233cc0 next=7fbcda234198 (x=1717, y=339, w=1972, h=1290) ink-overflow=(x=-240, y=-240, w=2452, h=1770) scr-overflow=(x=0, y=0, w=1972, h=1290) [content=7fbcda106020] [cs=7fbcda241c58] <
Block(button)(3)@7fbcda233fe0 parent=7fbcda233f28 (x=360, y=180, w=1252, h=930) [content=7fbcda106020] [cs=7fbcda242798:-moz-button-content] <
line@7fbcda234148 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,before:nobr,after:nobr[0x100] (x=0, y=0, w=1252, h=930) <
Text(0)"btn"@7fbcda2340a8 parent=7fbcda233fe0 (x=0, y=0, w=1252, h=930) [content=7fbcda102600] [cs=7fbcda242888:-moz-text] [run=7fbcddf0e100][0,3,T]
>
>
>
|
|
Assignee | |
Comment 19•3 years ago
|
||
Daniel, I believe your patch in bug 1672640 comment 11 or the one in bug 793456 can let us get correct baseline for <button>. Bug 1672640 may be a better solution because it also fixed the <input>'s baseline . (Bug 793456 is obsolete I think). I'm making this bug depending on bug 1672640.
|
|
||
Comment 20•3 years ago
|
||
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/7ead777c03ed Don't cache ascent in CachedBAxisMeasurement. r=dholbert
|
||
Comment 21•3 years ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
|
||
Comment 23•3 years ago
|
||
Old issue and not seeing any dupes, so it's not clear to me how much this is affecting us in the wild. Happy to consider uplift requests if we think it's needed, though.
|
|
||
Updated•3 years ago
|
Description
•