1227525 - Assertion failure: "We gave button-contents frame unconstrained available height, so it should be complete", with table-cell, float, & vertical writing-mode

Status: RESOLVED WORKSFORME

(Core :: Layout: Floats, defect)

Description

[Jesse Ruderman]

Attachment: testcase

The assertion was added in:
https://hg.mozilla.org/mozilla-central/rev/7164978f006d

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Jesse Ruderman]

Assertion failure: (0 == ((contentsReflowStatus) & 0x1)) (We gave button-contents frame unconstrained available height, so it should be complete), at layout/forms/nsHTMLButtonControlFrame.cpp:331

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 2

(Slightly tweaked testcase, removing a few small unnecessary bits.)

Comment 4

[Daniel Holbert [:dholbert]]

I'll bet this is related to bug 1227616 (on table-cells & vertical writing-modes), or bug 1077521 more generally.

Based on the testcases here, it seems likely that the table-cell has a tall child with a vertical writing-mode, which ends up fragmenting (incomplete), and the table-cell likely passes that incomplete reflow-status up to the button, which surprises the button because it provided unconstrained height. Or something like that.

For now, I suppose we should soften this assertion to be non-fatal, since I doubt we'll have a quick/easy fix here, and I don't want to block fuzzers by having them abort on this.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (with flex container instead of button)

Flexbox has a similar fatal assertion, which this modified testcase (swapping in a flex container for the button) triggers.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: band-aid: use nonfatal assertions (so fuzzers can proceed)

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Frames splitting when the parent doesn't expect them to is pretty bad; we shouldn't paper over it by weakening the assertions.

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 8691496 [details] [diff] [review]
band-aid: use nonfatal assertions (so fuzzers can proceed)

OK, canceling review then.

Comment 9

[Jesse Ruderman]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> Created attachment 8691492 [details]
> testcase 3 (with flex container instead of button)
> 
> Flexbox has a similar fatal assertion, which this modified testcase
> (swapping in a flex container for the button) triggers.

I think bug 1145743 covers that one. Will they be fixed by the same patch?

Comment 10

[Daniel Holbert [:dholbert]]

Possibly, yes (if the patch ends up fixing things on the table/table-cell side of things, which I think it needs to).

Comment 11

[Mats Palmgren (inactive)]

All testcases here WFM on Linux.

Comment 12

[Timea Cernea [:tbabos][inactive]]

Both test cases WFM on macOS 10.15 on the latest Firefox Release 85.0.2 (64-bit). Based on this and Comment 11, closing this as resolved:worksforme.