Last Comment Bug 522374 - "ASSERTION: Creating ContinuingTextFrame, but there is no more content" with image map, bidi
: "ASSERTION: Creating ContinuingTextFrame, but there is no more content" with ...
Status: RESOLVED FIXED
: [sg:critical?]
: assertion, fixed1.9.0.16, testcase
Product: Core
Classification: Components
Component: Layout
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla1.9.2
Assigned To: Timothy Nikkel (:tn)
: layout
:
:
: 377438
  Show dependency treegraph
 
Reported: 2009-10-14 16:03 PDT by Jesse Ruderman
Modified: 2010-04-06 17:15 PDT (History)
8 users (show)
roc: wanted1.9.2+
dveditz: blocking1.9.0.16+
dveditz: wanted1.9.0.x+
tnikkel: in‑testsuite+
See Also:
Crash Signature:
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  beta2-fixed
  .6+
  .6-fixed


Attachments
testcase (731 bytes, text/html)
2009-10-14 16:03 PDT, Jesse Ruderman
no flags Details
testcase without bidi (731 bytes, text/html)
2009-10-15 12:39 PDT, Simon Montagu
no flags Details
patch (1.27 KB, patch)
2009-10-16 16:35 PDT, Timothy Nikkel (:tn)
bzbarsky: review+
dveditz: superreview+
roc: approval1.9.2+
dveditz: approval1.9.1.6+
dveditz: approval1.9.0.16+
Details | Diff | Splinter Review

Summon comment box

Description Jesse Ruderman 2009-10-14 16:03:45 PDT
Created attachment 406335 [details]
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 2009-10-15 12:39:37 PDT
Created attachment 406512 [details]
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 2009-10-15 12:46:34 PDT
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 (:tn) 2009-10-16 16:35:42 PDT
Created attachment 406810 [details] [review]
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 (:bz) 2009-10-16 17:15:40 PDT
Comment on attachment 406810 [details] [review]
patch

r=bzbarsky.  Nice catch!
Comment 5 Boris Zbarsky (:bz) 2009-10-16 17:17:45 PDT
Probably need this on branches too, right?
Comment 6 Timothy Nikkel (:tn) 2009-10-16 17:19:51 PDT
Yes, the same code is on all branches, so it should all get fixed.
Comment 7 Boris Zbarsky (:bz) 2009-10-16 21:48:13 PDT
You should probably request 1.9.1 and 1.9.0 approvals as well...
Comment 8 Daniel Veditz 2009-10-19 14:30:24 PDT
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 2009-10-23 10:32:41 PDT
Is there any reason this hasn't landed on mozilla-central for testing yet?
Comment 10 Timothy Nikkel (:tn) 2009-10-24 02:13:50 PDT
http://hg.mozilla.org/mozilla-central/rev/45f949dd02a5
Comment 11 Timothy Nikkel (:tn) 2009-10-31 00:49:20 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2fcdc42c7336
Comment 12 Daniel Veditz 2009-11-02 14:45:09 PST
Comment on attachment 406810 [details] [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 (:tn) 2009-11-09 00:59:11 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c2a31f54e224
Comment 14 Boris Zbarsky (:bz) 2009-11-09 09:18:42 PST
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] 2009-11-20 11:40:42 PST
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] 2009-11-20 11:44:33 PST
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.

Note You need to log in before you can comment on or make changes to this bug.