This testcase triggers some textframe assertions. In asciibetical order: ###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69 ###!!! ASSERTION: Bad offset: 'aPos <= aFrag->GetLength()', file layout/generic/nsTextFrameThebes.cpp, line 512 ###!!! ASSERTION: Content offset/length out of bounds: 'offset + limitLength == PRInt32(frag->GetLength())', file layout/generic/nsTextFrameThebes.cpp, line 6242 ###!!! ASSERTION: Creating ContinuingTextFrame, but there is no more content: 'mContentOffset < PRInt32(aContent->GetText()->GetLength())', file layout/generic/nsTextFrameThebes.cpp, line 3508 ###!!! ASSERTION: No text for IsSpace!: 'aLength > 0', file layout/generic/nsTextFrameThebes.cpp, line 540 ###!!! ASSERTION: No text for IsSpace!: 'aPos < aFrag->GetLength()', file layout/generic/nsTextFrameThebes.cpp, line 558 ###!!! ASSERTION: Should have been cleared: 'mBreakSinks.IsEmpty()', file layout/generic/nsTextFrameThebes.cpp, line 649 ###!!! ASSERTION: Should have been cleared: 'mLineBreakBeforeFrames.IsEmpty()', file layout/generic/nsTextFrameThebes.cpp, line 651 ###!!! ASSERTION: Should have been cleared: 'mMappedFlows.IsEmpty()', file layout/generic/nsTextFrameThebes.cpp, line 652 ###!!! ASSERTION: bad index: 'PRUint32(aIndex) < mState.mLength', file nsTextFragment.h, line 186 ###!!! ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file nsTextFrame.h, line 319

I have a patch for this inside Bidi resolution that "works", i.e. prevents the assertions and leaves the frame tree in a sane state, but I think the root cause of the bug is happening somewhere else when the content changes, before Bidi resolution takes place. After bidi resolution happens on the original text, after the appendChild() but before the removeChild(), the frame tree looks like this: Block(select)(0)@0xaeb05198 next=0xaeb05530 [content=0xb361f240] {180,180,1740,1020} [state=00000400] sc=0xaeb03fb8(i=1,b=0) pst=:-moz-display-comboboxcontrol-frame< line 0xaeb05250: count=2 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x10141] {240,60,480,900} < Text(-1)@0xaeb05208[0,2,F] next=0xb14c81c8 next-continuation=0xb14c81c8 {240,60,480,900} [state=90620000] [content=0xb2842c70] sc=0xaeb05140 pst=:-moz-non-element< "B " > Text(-1)@0xb14c81c8[2,1,T] prev-continuation=0xaeb05208 {0,0,0,0} [state=00020402] [content=0xb2842c70] sc=0xaeb05140 pst=:-moz-non-element< "\u064a" > > > Then after the removeChild, before bidi re-resolution, it looks like this: Block(select)(0)@0xaeb05198 next=0xaeb05530 [content=0xb361f240] {180,180,1500,1020} [state=00000400] sc=0xaeb03fb8(i=1,b=0) pst=:-moz-display-comboboxcontrol-frame< line 0xaeb05250: count=2 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x10100] {240,60,1260,900} < Text(-1)@0xaeb05208[0,2,F] next=0xb14c81c8 next-continuation=0xb14c81c8 {240,60,720,900} [state=90220000] [content=0xb2842c70] sc=0xaeb05140 pst=:-moz-non-element< "\u064a\u5a5a" > Text(-1)@0xb14c81c8[2,-1,T] prev-continuation=0xaeb05208 {960,60,540,900} [state=80420000] [content=0xb2842c70] sc=0xaeb05140 pst=:-moz-non-element< "" > > > In other words, the content has changed, but the offsets in the frames still reflect the old content. The interesting thing is that the same content appears twice in the frame tree, once as copied here, directly below the select, and again further down in a "selectPopupList". When bidi resolution happens on the block in the selectPopupList after the removeChild, the content seems to have been reframed somehow: Block(option)(0)@0xaeb03d98 [content=0xb37f1600] {0,0,2940,900} [state=00005000] sc=0xaeb03be0(i=1,b=0)< line 0xaeb03ef0: count=1 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8141] {180,0,1260,900} < Text(0)@0xb14c8180[0,1,T] {900,0,540,900} [state=c0424000] [content=0xb3625a60] sc=0xaeb03e00 pst=:-moz-non-element< "\u064a" So I think the real fix would be to make the same reframing happen on both copies of the content, but I don't know where it happens.

The problem is probably that nsComboboxControlFrame::ActuallyDisplayText doesn't notify on the primary text frame. It should.

Simon, will you be able to work on this soon, to do the fix in comment #3? Or shall I assign it to someone else?

I did try to work on this, but either I didn't understand comment 3, or it didn't help

Turns out the underlying issue is that we forgot to set the primary frame for the mDisplayContent. So when the mDisplayContent text was changed, we never calls CharacterDataChanged on the frame.

Followup patch. Holding a reference to mTextFrame is dangerous and bogus if there are actually many text frames representing the node. Anyway it turns out mTextFrame is not actually needed, we can just use a local variable instead in CreateFrameFor.

The two patches are the same.

I hate that. Give me some HTML5 File API love!

mats, can you please review this?

It looks like nsCSSFrameConstructor::FindPrimaryFrameFor used to do this for us "on demand", so I'm guessing this bug is a regression from removing the primary frame map (and thus does not affect 1.9.2 or older)

Comment on attachment 437984 [details] [diff] [review] fix Looks good but you need to fix nsGfxButtonControlFrame::CreateFrameFor in the same way. Also, nsCSSFrameConstructor::CreateAnonymousFrames should probably assert that content->GetPrimaryFrame() is non-null on line 3887, i.e. when CreateFrameFor() returns a non-null frame. I don't think they need to be the same though. http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp#3859 r=mats with that addressed

Thanks, sounds good. Yes, they don't need to be the same.

Can we get a call as to whether or not this as an sg:crit?

I think it should be, since all kinds of weirdness can happen. No point in agonizing over it since we have a patch.

Comment on attachment 437984 [details] [diff] [review] fix sr=dbaron

I checked this in: http://hg.mozilla.org/mozilla-central/rev/489f29b06ef5 http://hg.mozilla.org/mozilla-central/rev/4af9e6180b76 I backed out the nsGfxButtonControlFrame change because it broke accessibility tests: http://hg.mozilla.org/mozilla-central/rev/a908be341373 and filed bug 565569 on fixing nsGfxButtonControlFrame.

Crashtest: http://hg.mozilla.org/mozilla-central/rev/ef75056ce708