476087 - [FIX]"ASSERTION: Someone screwed up the indexing" with execCommand("indent") on td

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: out of bounds: 'PRInt32(aIndex) >= 0 && aIndex <= l', file /Users/jruderman/central/layout/base/nsChildIterator.h, line 132

###!!! ASSERTION: Someone screwed up the indexing: '*iter == aChild', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 7897

###!!! ASSERTION: anonymous nodes should not be in child lists: '!aOldChild->IsRootOfAnonymousSubtree()', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 12377

The second assertion was added in bug 307394 part 2.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yeah.  These are all about the same thing, actually.  In this case, there's native anonymous content that nsHTMLEditor::CreateAnonymousElement calls RecreateFramesFor() on.  This causes use to post that third assert, end up in a RecreateFramesForContent for the native anon content, get -1 as the index, and happily use that.  There's no XBL involved, so we follow the "normal content" codepath.

Now the issue, of course, is that recreating frames for native anonymous content is simply not supported and never has been.  I hate hacks like this.  :(  I thought we had a bug on editor being broken here, but I can't find it offhand.

I can make those first two asserts go away by just restoring the behavior we used to have here, which was to blindly append.  In fact that's still the behavior, due to the bounds-checks in nsContentIterator.  But really, the caller is broken and should be fixed.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Avoid the new asserts

Comment 3

[Jesse Ruderman]

It might be good to file a bug on the editor brokenness and mention the bug number in the comment.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Like I said in comment 1, I'm pretty sure we have a bug on that already.  I'll look for it, but if someone else finds it first, so much the better.  Once we find it, I'll add it to the comments.

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 360031 [details] [diff] [review]
Avoid the new asserts

r+sr=dbaron, but maybe add an NS_WARNING in a final else?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/6f89dd8e23af

I filed bug 476670 because I can't seem to find the existing one.