1140198 - Crash [@ nsCSSFrameConstructor::WipeContainingBlock] with display:contents

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (with contenteditable)

###!!! ASSERTION: Must have non-inline, non-ib-split, non-pseudo frame as root (or child of root, for a table root)!: 'aContainingBlock', file layout/base/nsCSSFrameConstructor.cpp, line 12120

Followed by a crash [@ nsCSSFrameConstructor::WipeContainingBlock]

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Mats Palmgren (inactive)]

Attachment: fix+test

We currently enforce a block frame for the root, but we didn't fixup
the 'display' style.  The CSS spec says we should do that, which is
nice because it also fixes the crash. :-)

The problem was that we never found a block ancestor in this loop:
http://hg.mozilla.org/mozilla-central/annotate/7d85ac833cff/layout/base/nsCSSFrameConstructor.cpp#l12119
because "IsInlineOutside()" is true for display:contents.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a0d2c2ffbdb

Comment 3

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27ac269bb796

Comment 4

[Mats Palmgren (inactive)]

Sorry, wrong link above - here's the correct link:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa19896bead7

Comment 5

[Phil Ringnalda (:philor)]

There's a test, layout/style/test/test_root_node_display.html (in mochitest-5 on desktop), which tests that the result of this is not true, "root node should make 'display:contents' compute to the same value that it computes to on a floated element - got block, expected contents", now permafailing.

Comment 6

[Mats Palmgren (inactive)]

My bad.  I'll land a small tweak to that test for now to make it green again.

Comment 7

[Mats Palmgren (inactive)]

Fixed the failing test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/99a276974d6d

Comment 8

[Mats Palmgren (inactive)]

David, can you confirm I'm reading the spec correctly here?

http://dev.w3.org/csswg/css-display/#transformations  (last paragraph)
"The root element’s display type is always blockified. Additionally, a display-outside of contents computes to block-level on the root element."

That's what my patch here tries to implement.

The spec also says (first paragraph),
"Some layout effects require blockification or inlinification
of the box type, causing the box’s display-outside property, if it is not none or contents, to compute to block-level or inline-level (respectively)."

which is why I intentionally didn't do this inside EnsureBlockDisplay,
since the spec explicitly excludes 'contents' there.

Comment 9

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

Yes, that seems right.  display:contents overrides those other things, but it's not allowed to override there being only one root.

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/27ac269bb796
https://hg.mozilla.org/mozilla-central/rev/aa19896bead7
https://hg.mozilla.org/mozilla-central/rev/99a276974d6d