1140160 - "ASSERTION: Unexpected parent" and "MOZ_CRASH(nsFieldSetFrame::InsertFrames not supported)"

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Unexpected parent: 'parentFrame->GetType() != nsGkAtoms::fieldSetFrame', file layout/base/nsCSSFrameConstructor.cpp, line 7142

Hit MOZ_CRASH(nsFieldSetFrame::InsertFrames not supported) at layout/forms/nsFieldSetFrame.cpp:661

Even the crash is debug-only (it's in a big ifdef debug).

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Jesse Ruderman]

I think roc added that MOZ_CRASH.

Comment 3

[Mats Palmgren (inactive)]

I can take a look since it's likely caused by the display:contents.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Something somewhere is missing a GetContentInsertionFrame(), clearly..

Comment 5

[Mats Palmgren (inactive)]

(In reply to Not doing reviews right now from comment #4)
> Something somewhere is missing a GetContentInsertionFrame(), clearly..

Hmm, nsFieldSetFrame doesn't have a GetContentInsertionFrame() method and
I don't think it ever did.  I think bug 496840 is what changed frame construction
so that nsFieldSetFrame::Insert/Append/RemoveFrames wouldn't be called
at all.  Before that change, I think it was called only for legend frames.
(bug 931464 later removed the code and made those methods assert if called)

Anyway, what normally happens is: GetInsertionPoint would flag this
as a "multiple insertion points" situation:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp?rev=c6c310429562#8911

Then we'll either re-create the whole fieldset frame subtree:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp?rev=c6c310429562#7579

or do an insertion, which leads to GetInsertionPrevSibling, which has a side-effect
of also fixing up the parent frame for fieldsets by calling GetAdjustedParentFrame:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp?rev=c6c310429562#6596
which returns the right parent depending on the child:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp?rev=c6c310429562#6488

Comment 6

[Mats Palmgren (inactive)]

Attachment: fix+tests

This adds a GetAdjustedParentFrame on the relevant display:contents path
in GetInsertionPrevSibling.  I also clarified a comment that's a bit
confusing.

(there's still an issue that <legend>s don't get the right frame type
in this situation, but it's a separate issue so I filed bug 1140579)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d49ebb63997

Comment 7

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3fef33215d39

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3fef33215d39