1552357 - Incorrect intrinsic size calculation on wrapper for inline-level elements with padding and `box-decoration-break: clone`

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P3)

Description

[Nicolas Hoizey]

Attachment: Screenshot of the reduced test case in Firefox 66.0.5

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

• I want to present a list using Flexbox.
• Each item of the list contains links.
• I want some padding arround (left and right) these links.
• Because I use box-shadow (and text-shadow) to create beautiful links, I use box-decoration-break: clone.

Actual results:

The lastest link in a list item wraps under the others even if there's still plenty of space available on the right.

Expected results:

All links inside one list item should be on the same line if there's enough space for it.

A reduced test case on CodePen shows how Firefox behaves and that I have to either remove the padding or box-decoration-break to have the links rendered as I assume they should be (like in other browsers):
https://codepen.io/nhoizey/pen/JVwMLQ?editors=1100

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1

This seems to be a bug with intrinsic sizing -- it's not flex-specific.

(e.g. I can reproduce it on the original codepen if I adjust the ul rule to have width:max-content instead of display:flex.)

Here's a reduced testcase that just targets this with some unbreakable inline elements and an intrinsically sized container.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 2

Comment 3

[Daniel Holbert [:dholbert]]

With both attached reduced testcases:

EXPECTED RESULTS: Red border should encompass the yellow children.

ACTUAL RESULTS: Red border doesn't stretch far enough to the right -- it looks like it's only as wide as the first child plus the subsequent children's content-box-width. (It does not account for their padding.)

This is not a recent regression -- here's the regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1dc6b294800d&tochange=c45f6e6d6005

That includes our changeset that enabled support for box-decoration-break in Nightlies, in bug 1006326. (It landed in Firefox 33 and was uplifted to 32).

ni=mats who worked on that

Comment 4

[Daniel Holbert [:dholbert]]

Ah, also worth noting, Chrome needs a webkit prefix in order to undestand these styles -- so it needs -webkit-box-decoration-break: clone (which is not currently present in my reduced testcases)

If I add that style, though, it still doesn't affect Chrome's rendering.

Comment 5

[Daniel Holbert [:dholbert]]

(It looks like the relevant spec text is https://drafts.csswg.org/css-break-3/#valdef-box-decoration-break-clone and I don't immediately see anything there that makes our behavior here make sense.)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1 (now with -webkit prefix)

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 2 (now with -webkit prefix)

Comment 8

[Nicolas Hoizey]

I don't know if it is relevant, but I find it strange that the bug is “fixed“ when I open the devtools, as shown here: https://twitter.com/nhoizey/status/1129113612654698496

Comment 9

[Emilio Cobos Álvarez (:emilio)]

That's because opening devtools causes a full re-layout of the page, which probably masks the bug.

Comment 10

[Daniel Holbert [:dholbert]]

Also, to clarify/narrow comment 8: opening devtools only masks the bug in the place where Nicolas initially noticed the bug, which is on https://nicolas-hoizey.com/2019/04/how-the-boeing-737-max-disaster-looks-to-a-software-developer.html per https://twitter.com/nhoizey/status/1129111589225938944

It doesn't mask the bug in the attached testcases or in comment 0's codepen.

Comment 11

[Mats Palmgren (inactive)]

Attachment: Bug 1552357 - Improve intrinsic sizing of inlines. r=jfkthame

This is primarily to fix sizing of 'box-decoration-break:clone' inlines,
but also some 'slice' edge cases by recognizing more break opportunities,
and to improve sizing when BIDI-continuations are involved.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/750046b87c63
Improve intrinsic sizing of inlines.  r=jfkthame

Comment 13

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

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

Comment 14

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

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/750046b87c63

Comment 16

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

Upstream PR merged