522374 - "ASSERTION: Creating ContinuingTextFrame, but there is no more content" with image map, bidi

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

This testcase triggers

###!!! ASSERTION: Creating ContinuingTextFrame, but there is no more content: 'mContentOffset < PRInt32(GetFragment()->GetLength())', file /Users/jruderman/central/layout/generic/nsTextFrameThebes.cpp, line 3518

and a bundle of other assertions.

Comment 1

[Simon Montagu :smontagu]

Attachment: testcase without bidi

Most of the assertions in the original testcase are triggered during bidi resolution, but the frame tree seems to be already wonky before bidi resolution starts. Even without bidi, as in this version of the test case, there is still ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file /Users/simon/mozwork/hgtree/mozilla/layout/generic/nsTextFrame.h, line 323

Comment 2

[Simon Montagu :smontagu]

Frame dump at the beginning of bidi resolution in the first testcase (with some of the repetitious assertion spew snipped):

           Block(body)(1)@0x1f5aff8 {480,480,58200,2304} [state=00101000] sc=0x1f5aee0(i=1,b=1)<
            line 0x1eef390: count=1 state=inline,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8001] {0,0,0,0} <
              Text(0)@0x1eef348[0,4,T]  next=0x1f5b190 {0,0,0,0} [state=00000402] [content=0x10d7e880] sc=0x1f5a2b8 pst=:-moz-non-element<
                "  \u062a "
              >
            >
            line 0x1f5b7b0: count=1 state=block,dirty,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8049] {0,0,60,2304} ca={0,0,426,2304} <
              Block(div)(1)@0x1f5b190 {0,0,60,2304} [state=00100000] [overflow=0,0,426,2304] sc=0x1f5afa0(i=2,b=1)<
                line 0x1f5b760: count=2 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x10120] {0,0,300,1152} <
                  ImageFrame(img)(0)@0x1f5bc60 {0,516,300,300} [state=00200000] [content=0x10d7e920] [src=data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR42mP4z8AAAAMBAQD3A0FDAAAAAElFTkSuQmCC]
                  Text(1)@0x1eef268
###!!! ASSERTION: bad index: 'PRUint32(aIndex) < mState.mLength', file ../../dist/include/nsTextFragment.h, line 186
                    [0,9,F]  next=0x1eef2b0 next-continuation=0x1eef2b0 {300,96,0,960} [state=c1400000] [content=0x10d7f590] sc=0x1f5b3a8 pst=:-moz-non-element<
                    " b       "
                  >
                >
                line 0x1eef2f8: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8100] {0,1152,426,1152} <
                  Text(1)@0x1eef2b0
###!!! ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file /Users/simon/mozwork/hgtree/mozilla/layout/generic/nsTextFrame.h, line 323
Program received signal:  “SIGTRAP”.
                   [9,-6,T]  next=0x1f5b638 prev-continuation=0x1eef268 {0,1248,426,960} [state=90600004] [content=0x10d7f590] sc=0x1f5b3a8 pst=:-moz-non-element<
                    ""
                  >
                >

Comment 3

[Timothy Nikkel (:tnikkel)]

Attachment: patch

Indeed this has nothing to do with bidi; the frame tree gets screwed up the first time we mutate the tree: when we execute |area.nextSibling.data += " a "|.

GetInsertionPrevSibling calls FindPreviousSibling which calls FindFrameForContentSibling. The previous content sibling is the area element, and the primary frame for it is the image frame. So the image frame becomes the previous sibling and we change the parent frame to use for insertion to the parent of the image frame, which is the div with id main. So the text gets inserted in the wrong place in the frame tree, and things get worse from there.

This patch fixes all asserts in both testcases.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 406810 [details] [diff] [review]
patch

r=bzbarsky.  Nice catch!

Comment 5

[Boris Zbarsky [:bzbarsky]]

Probably need this on branches too, right?

Comment 6

[Timothy Nikkel (:tnikkel)]

Yes, the same code is on all branches, so it should all get fixed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

You should probably request 1.9.1 and 1.9.0 approvals as well...

Comment 8

[Daniel Veditz [:dveditz]]

If this isn't blocking 1.9.2 this probably doesn't "block" the older branches, but we'll look at the approvals anyway.

Comment 9

[Daniel Veditz [:dveditz]]

Is there any reason this hasn't landed on mozilla-central for testing yet?

Comment 10

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/45f949dd02a5

Comment 11

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2fcdc42c7336

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 406810 [details] [diff] [review]
patch

sr=dveditz to satisfy the policy which apparently doesn't have room for exceptions for trivial patches.

Approved for 1.9.1.6 and 1.9.0.16, a=dveditz for release-drivers

Comment 13

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c2a31f54e224

Comment 14

[Boris Zbarsky [:bzbarsky]]

Checking in nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v  <--  nsCSSFrameConstructor.cpp
new revision: 1.1489; previous revision: 1.1488

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Look at in 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.6pre) Gecko/20091119 Shiretoko/3.5.6pre using attached testcases. 

I do not see the assertions but when checking with a 1.9.1.4 debug build, I don't see the assertions there either. Do these show up in 1.9.1?

Comment 16

[Al Billings [:abillings - ex-MoCo]]

I see the same thing when I look at the testcases in 1.9.0 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.0.16pre) Gecko/2009111916 GranParadiso/3.0.16pre and my 1.9.0.15 debug build. The assertions don't show up.

Comment 17

[Timothy Nikkel (:tnikkel)]

Added crashtest to 1.9.1, 1.9.2, and mozilla-central.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/976ec8c4d911
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/914ae05fe0a2
http://hg.mozilla.org/mozilla-central/rev/260d768e55f4