409565 - "ASSERTION: unexpected frame type" with -moz-column, float, svg

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase

Loading the testcase triggers:

###!!! ASSERTION: unexpected frame type: 'Not Reached', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10742

###!!! ASSERTION: reflow dirty lines failed: 'NS_SUCCEEDED(rv)', file /Users/jruderman/trunk/mozilla/layout/generic/nsBlockFrame.cpp, line 937

Comment 1

[Boris Zbarsky [:bzbarsky]]

We're trying to create a continuation for a nsSVGOuterSVGFrame, which of course fails.

The real issue is that we're trying to SplitPlaceholder on a placeholder whose out-of-flow is not splittable.  That seems like something we'd want to avoid doing...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Then again, I guess any container frame claims to be splittable by default... Should SVG outer frames be overriding GetSplittableType() or some such?

Comment 3

[fantasai]

If they're not splittable, then yeah. SVG frames probably shouldn't be inheriting nsSplittableFrame at all if they're not splittable. Don't know how risky it would be to decouple nsContainerFrame from nsSplittableFrame, but it would also save us 8 bytes per SVG frame...

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: patch

bz: I should think so.

fantasai: it is this late in the game.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 304895 [details] [diff] [review]
patch

>+  nsSplittableType GetSplittableType() const;

virtual, right?  With that, looks good.

Please file a followup bug to do the save-space thing?

Comment 6

[Jonathan Watt [:jwatt]]

Comment on attachment 304895 [details] [diff] [review]
patch

Yup, it stays virtual of course, but better to keep form and declare it as you say.

I filed bug 418964 for the space saving.

Comment 7

[Jonathan Watt [:jwatt]]

Meant to say I've added the 'virtual' locally - I'll check it in with that.

Comment 8

[Jonathan Watt [:jwatt]]

Checked in.

Comment 9

[Jesse Ruderman]

Crashtest checked in.