397844 - Assertions / crash [@ nsFrameList::InsertFrame] with rtl, :first-letter, wrapping, padding

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase 1 (crashes firefox when loaded)

Loading testcase 1 triggers three assertions, and a crash.

###!!! ASSERTION: child list is not empty for initial reflow: 'mFrames.IsEmpty()', file /Users/jruderman/trunk/mozilla/layout/generic/nsInlineFrame.cpp, line 318

###!!! ASSERTION: unexpected flow: 'mFrames.ContainsFrame(nextInFlow)', file /Users/jruderman/trunk/mozilla/layout/generic/nsInlineFrame.cpp, line 462

###!!! ASSERTION: StealFrame failure: 'NS_SUCCEEDED(rv)', file /Users/jruderman/trunk/mozilla/layout/generic/nsContainerFrame.cpp, line 1026

The crash appears to be a null dereference [@ nsFrameList::InsertFrame].

This is similar to bug 389630, which is fixed.  That bug had some of the same assertions, a somewhat similar stack trace for the crash, and a testcase using most of the same features.

Filing as security-sensitive for reasons that I'll make clearer in a few minutes.

Comment 1

[Jesse Ruderman]

Attachment: testcase 2

When this testcase is reloaded, it triggers:

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 673

Comment 2

[Jesse Ruderman]

Security-sensitive for general scariness (similar to a bug with an sg:critical crash, triggers the FrameArena assertion).  I haven't seen any direct evidence that this bug can lead to scary crashes.

Comment 3

[Jesse Ruderman]

Attachment: reftest: testcase

The incorrect-layout issue is much easier to trigger than the assertions and crash.

Comment 4

[Jesse Ruderman]

Attachment: reftest: reference

Comment 5

[Daniel Holbert [:dholbert]]

Testcase #1 and #2 work fine for me under Linux w/ a current trunk build.  (I don't get the crash / assertions mentioned in Comment 1 and Comment 2)

However, the reftest still doesn't match -- "bc" is wrapped to its own line.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to comment #5)
Despite working on Linux, testcase #1 and #2 are still broken on current Mac trunk builds.

So, those testcases are Mac-specific.  The posted reftest is broken on all platforms, though.

Comment 7

[Jesse Ruderman]

Attachment: testcase 3 (crashes firefox when loaded)

Let's see if switching to a monospace font and |ch| units makes the testcase cross-platform.  On Mac, this has the same effect as testcase 1.

Comment 8

[Daniel Holbert [:dholbert]]

The (approved) patch posted for Bug 399384 fixes this bug.  Just waiting for the tree to re-open....

Comment 9

[Daniel Holbert [:dholbert]]

Fixed by checkin for bug 399384.

Comment 10

[Jesse Ruderman]

These testcases don't seem to cause any real problems on the 1.8 branch.

Comment 11

[Jesse Ruderman]

I checked in "testcase 2" and "testcase 3" as crashtests.  I also checked in the reftest.

Comment 12

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).