673770 - "ASSERTION: prev sibling not in line list" with -moz-column, abs pos, huge font size

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Among the usual boring assertions about heights and stuff, which are covered by bug 569193 and bug 575011:

###!!! ASSERTION: Column set should be complete if the available height is unconstrained: 'NS_FRAME_IS_FULLY_COMPLETE(aStatus) || aReflowState.availableHeight != NS_UNCONSTRAINEDSIZE', file layout/generic/nsColumnSetFrame.cpp, line 1078

###!!! ASSERTION: Shouldn't be incomplete if availableHeight is UNCONSTRAINED.: 'aReflowState.availableHeight != NS_UNCONSTRAINEDSIZE', file layout/generic/nsBlockFrame.cpp, line 1456

There's one scary assertion that doesn't appear in any other open bugs:

###!!! ASSERTION: prev sibling not in line list: 'Not Reached', file layout/generic/nsBlockFrame.cpp, line 4841

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Daniel Veditz [:dveditz]]

roc: please find someone on your team to investigate whether we need to worry about this.

Comment 3

[Mats Palmgren (inactive)]

Attachment: stack + frame dumps

I don't think we need to worry about this.

The prev-sibling frame given by nsCSSFrameConstructor::ContentRangeInserted 
happens to be on the "Overflow-lines" list.
nsBlockFrame::AddFrames only checks 'mLines':
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp#4834
There is a bit of defensive programming for this case though that disregards
the prev-sibling frame so the frames are prepended to the frame list.
I've included a dump of the resulting frame tree.

I don't think anything bad can happen other than some content likely
being misplaced in the rendered page.

We should probably make nsBlockFrame::AddFrames check overflow lines
for a correct fix...

Comment 4

[Mats Palmgren (inactive)]

Attachment: check overflow lines too

"lineList->back()->LastChild()" is a linear search but there's no way
to avoid that since there's no nsFrameList backing the overflow list.
It looks a bit silly to do that and then search backwards using
RFindLineContaining, but we need a frame list later for the InsertFrames
which means we have to find that last frame anyway.

Comment 5

[Mats Palmgren (inactive)]

Attachment: crashtest

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 550083 [details] [diff] [review]
check overflow lines too

Review of attachment 550083 [details] [diff] [review]:
-----------------------------------------------------------------

I assume you'll include the test...

Comment 7

[Mats Palmgren (inactive)]

Landed on inbound with test:
http://hg.mozilla.org/integration/mozilla-inbound/rev/04a12efc218c
http://hg.mozilla.org/integration/mozilla-inbound/rev/7ceb1468a822

Comment 8

[Mats Palmgren (inactive)]

The defensive code in nsBlockFrame::AddFrames is present in all versions
back to 1.9.1 (at least), so I think we can open this bug.
http://mxr.mozilla.org/mozilla1.9.1/source/layout/generic/nsBlockFrame.cpp#4838

Comment 9

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/rev/04a12efc218c
http://hg.mozilla.org/mozilla-central/rev/7ceb1468a822

Comment 10

[Mats Palmgren (inactive)]

I don't think this is needed on any branches, see comment 3 and comment 8.

Comment 11

[Alex Keybl [:akeybl]]

Thanks Mats. status1.9.2->unaffected

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to Mats Palmgren [:mats] from comment #8)
> The defensive code in nsBlockFrame::AddFrames is present in all versions
> back to 1.9.1 (at least), so I think we can open this bug.

dveditz / akeybl, are we OK to open this bug?  (It was fixed in Firefox 8.)

Comment 13

[Mats Palmgren (inactive)]

I don't see anything security-sensitive about this bug now.