Last Comment Bug 466763 - Crash [@ nsCSSFrameConstructor::ConstructFrame] with RTL, floating first-letter
: Crash [@ nsCSSFrameConstructor::ConstructFrame] with RTL, floating first-letter
Status: VERIFIED FIXED
[sg:critical]
: crash, testcase, verified1.9.0.12, verified1.9.1
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.9.2a1
Assigned To: Simon Montagu :smontagu
:
:
Mentors:
Depends on: 468491
Blocks: stirdom 401621 413085
  Show dependency treegraph
 
Reported: 2008-11-25 19:53 PST by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
7 users (show)
smontagu: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (446 bytes, text/html)
2008-11-25 19:53 PST, Jesse Ruderman
no flags Details
Patch (3.10 KB, patch)
2008-11-27 08:21 PST, Simon Montagu :smontagu
roc: review+
roc: superreview+
mbeltzner: approval1.9.1+
Details | Diff | Splinter Review
patch backported to 1.9.0.x (3.50 KB, patch)
2009-06-11 15:42 PDT, Daniel Holbert [:dholbert]
dveditz: approval1.9.0.12+
Details | Diff | Splinter Review

Description Jesse Ruderman 2008-11-25 19:53:32 PST
Created attachment 350104 [details]
testcase

Thread 0 Crashed:
0   0xdddddddd
1   nsCSSFrameConstructor::ConstructFrame(nsFrameConstructorState&, nsIContent*, nsIFrame*, nsFrameItems&) + 89 (nsCSSFrameConstructor.cpp:7379)
2   nsCSSFrameConstructor::ContentAppended(nsIContent*, int) + 2090 (nsCSSFrameConstructor.cpp:8580)
3   PresShell::ContentAppended(nsIDocument*, nsIContent*, int) + 279 (nsPresShell.cpp:4698)
4   nsNodeUtils::ContentAppended(nsIContent*, int) + 324 (nsNodeUtils.cpp:119)
5   nsGenericElement::doInsertChildAt(nsIContent*, unsigned int, int, nsIContent*, nsIDocument*, nsAttrAndChildArray&) + 1669 (nsGenericElement.cpp:3265)
6   nsGenericElement::InsertChildAt(nsIContent*, unsigned int, int) + 127 (nsGenericElement.cpp:3193)
7   nsGenericElement::doReplaceOrInsertBefore(int, nsIDOMNode*, nsIDOMNode*, nsIContent*, nsIDocument*, nsIDOMNode**) + 4185 (nsGenericElement.cpp:3929)
8   nsGenericElement::InsertBefore(nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) + 63 (nsGenericElement.cpp:3512)
9   nsHTMLSpanElement::InsertBefore(nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) + 38 (nsHTMLSpanElement.cpp:57)
10  nsGenericElement::AppendChild(nsIDOMNode*, nsIDOMNode**) + 48 (nsGenericElement.h:497)
11  nsHTMLSpanElement::AppendChild(nsIDOMNode*, nsIDOMNode**) + 31 (nsHTMLSpanElement.cpp:57)
12  nsIDOMNode_AppendChild(JSContext*, unsigned int, long*) + 488 (dom_quickstubs.cpp:2339)
13  js_Interpret + 93406 (jsinterp.cpp:5118)
Comment 1 Simon Montagu :smontagu 2008-11-27 08:21:27 PST
Created attachment 350340 [details] [diff] [review]
Patch

What was happening here was that RemoveFloatingFirstLetterFrames deletes the first fluid continuation of the text frame child of the first letter frame, but does not delete its non-fluid continuations. When the first letter is right-to-left in a left-to-right paragraph, as here, this means that the bidi continuations are left hanging, and ContentAppended first identifies one of them as the parent frame for the new frames, and then removes the same frame in RemoveLetterFrames, leading to the crash.

The change in nsBlockFrame is for another crash that I came across while testing.

This also fixes bug 401621.
Comment 2 Simon Montagu :smontagu 2008-12-02 23:00:39 PST
http://hg.mozilla.org/mozilla-central/rev/6027c1cd6033
Comment 3 Mike Beltzner [:beltzner, not reading bugmail] 2008-12-29 14:46:43 PST
Comment on attachment 350340 [details] [diff] [review]
Patch

a191=beltzner
Comment 4 Simon Montagu :smontagu 2009-01-01 03:19:45 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/073269e99a2a
Comment 5 Henrik Skupin (:whimboo) 2009-02-05 03:47:12 PST
Verified fixed on trunk and 1.9.1 with builds on OS X and Windows:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204034421

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090204 Minefield/3.2a1pre ID:20090204032912
Comment 6 Daniel Holbert [:dholbert] 2009-06-11 15:42:05 PDT
Created attachment 382850 [details] [diff] [review]
patch backported to 1.9.0.x

Here's this bug's patch, backported to 1.9.0.x. (Just needed to fix bitrot in crashtests.list)

Requesting approval because this fixes bug 413085 which is blocking1.9.0.12+

(I'll also be posting a trivially-backported patch for bug 468491, the only listed regression from this bug.  That patch will go on that bug.)
Comment 7 Daniel Holbert [:dholbert] 2009-06-11 15:46:35 PDT
BTW, I've verified that the backported patch fixes the crash on this bug's testcase, as well as the crash on bug 413085's testcase, using a Linux debug build from CVS trunk.
Comment 8 Daniel Holbert [:dholbert] 2009-06-11 16:18:47 PDT
FWIW, I verified that the backported patch also fixes both testcases on bug 401621. They both crash immediately in an unpatched debug build, but they load fine (with no duplicated content) after patching.
Comment 9 Daniel Veditz [:dveditz] 2009-06-12 10:26:52 PDT
Comment on attachment 382850 [details] [diff] [review]
patch backported to 1.9.0.x

Approved for 1.9.0.12, a=dveditz for release-drivers
Comment 10 Daniel Holbert [:dholbert] 2009-06-12 12:29:21 PDT
Patch landed on CVS trunk:

Checking in layout/base/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/base/nsCSSFrameConstructor.cpp,v  <--  nsCSSFrameConstructor.cpp
new revision: 1.1484; previous revision: 1.1483
done
RCS file: /cvsroot/mozilla/layout/base/crashtests/466763-1.html,v
done
Checking in layout/base/crashtests/466763-1.html;
/cvsroot/mozilla/layout/base/crashtests/466763-1.html,v  <--  466763-1.html
initial revision: 1.1
done
Checking in layout/base/crashtests/crashtests.list;
/cvsroot/mozilla/layout/base/crashtests/crashtests.list,v  <--  crashtests.list
new revision: 1.100; previous revision: 1.99
done
Checking in layout/generic/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v  <--  nsBlockFrame.cpp
new revision: 3.961; previous revision: 3.960
done
Comment 11 Al Billings [:abillings] 2009-06-30 17:30:00 PDT
Verified for 1.9.0.12 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.12pre) Gecko/2009063005 GranParadiso/3.0.12pre (.NET CLR 3.5.30729). Testcase crashes 1.9.0.11 but does not crash 1.9.0.12.

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