526375 - [FIX]"ASSERTION: aParentFrame is not last?" with XUL, RLE

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: aParentFrame is not last?: 'nextSibling || !IsFrameSpecial(aParentFrame) || (IsInlineFrame(aParentFrame) && !GetSpecialSibling(aParentFrame) && !aParentFrame->GetNextContinuation())', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 5706

This assertion was added as part of mozilla-central changeset 822ba7ef29bb:

user:        Boris Zbarsky <[e-mail]>
date:        Fri Sep 18 14:00:21 2009 -0400
summary:     Bug 501847 part 4.  Change frame construction to create {ib} splits that have blocks wrapping runs of block kids and inlines wrapping runs of inline kids instead of wrapping all kids from the first block to the last block in a single block.  r=tn,dbaron,roc

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hmm.  What's failing here is the !aParentFrame->GetNextContinuation() test, because aParentFrame in fact has a next continuation.  And that next continuation is empty.  I guess that makes sense, since ContentAppended does GetLastContinuationWithChild(), not GetLastContinuation().

I think we can loosen up the assertion and make sure to call GetNextContinuation() for the aParentFrame when calling InsertFrames.

That said, how can I reliably reproduce this empty last continuation thing?  It could affect skip-sides stuff, and I want to make sure that we handle it right...

Comment 2

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

We really shouldn't have an empty continuation of a container element, apart from during reflow of course.

Comment 3

[Timothy Nikkel (:tnikkel)]

When normalize is called, the textnode for the "A" is in the first continuation of the inline, and the "\u202B" + "C" textnode is in the second (non-fluid) continuation. normalize() does a ContentRemoved on the "\u202B" + "C" textnode followed by a CharacterDataChanged on what was the "A" textnode in order to join the two textnodes into one. The ContentRemoved causes the second continuation of the inline to be empty, and it stays in the frame tree, empty, even after reflow and everything is flushed.

The same assert can be triggered without any BIDI continuations by just inducing a fluid continuation by adding enough text so that it line wraps. But as expected the empty continuation goes away after reflow.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> The ContentRemoved causes the second continuation of the inline to be empty

Right; I meant to mention that after comment 2 and apparently forgot... At the very least we can have empty continuations between the removal and when reflow happens.

But in that situation, is it ok to just put the new kids in the last continuation, instead of the last continuation that has kids?  The "has kids" check was added in bug 403369, it looks like.  Is the overflow container setup such that it's still relevant?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 6

[Boris Zbarsky [:bzbarsky]]

And to answer the question from comment 4, we do still need to look for last nonempty continuations, in general.

Comment 7

[Timothy Nikkel (:tnikkel)]

Do we want to append to an empty continuation if its a BIDI continuation?

Comment 8

[Boris Zbarsky [:bzbarsky]]

I don't know.  I do think that empty bidi continuations should go away, but that's a separate bug.

What we do NOT want to do, because it in fact causes test failures is append to an empty continuation for blocks in columnsets.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/707bb3e41f11