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

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: jruderman, Assigned: jwatt)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
assertion, testcase
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
Created attachment 294372 [details]
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
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...
Flags: blocking1.9?
Then again, I guess any container frame claims to be splittable by default... Should SVG outer frames be overriding GetSplittableType() or some such?
Component: Layout → SVG
QA Contact: layout → general

Comment 3

11 years ago
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...

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
(Assignee)

Comment 4

11 years ago
Created attachment 304895 [details] [diff] [review]
patch

bz: I should think so.

fantasai: it is this late in the game.
Attachment #304895 - Flags: review?(bzbarsky)
(Assignee)

Updated

11 years ago
OS: Mac OS X → All
Hardware: PC → All
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?
Attachment #304895 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

11 years ago
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.
Attachment #304895 - Flags: superreview?(roc)
(Assignee)

Comment 7

11 years ago
Meant to say I've added the 'virtual' locally - I'll check it in with that.
Assignee: nobody → jwatt
Attachment #304895 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 8

11 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 9

11 years ago
Crashtest checked in.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.