"ASSERTION: aPos out of range" and hang with bidi, :first-letter

VERIFIED FIXED

Status

()

Core
Layout: Text
--
critical
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: smontagu)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
Mac OS X
assertion, hang, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
Created attachment 375127 [details]
testcase (crashes Firefox when loaded)

###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file /Users/jruderman/central/gfx/thebes/src/gfxSkipChars.cpp, line 92

###!!! ASSERTION: aPos out of range: '0 <= aPos && aPos < mCharacterCount', file ../../../dist/include/thebes/gfxFont.h, line 857

Hang
(Assignee)

Comment 1

9 years ago
Same assertions as bug 489691
Assignee: nobody → smontagu
Blocks: 332655
(Reporter)

Comment 2

9 years ago
Same first assertion, at least.
(Assignee)

Comment 3

9 years ago
Created attachment 375189 [details] [diff] [review]
Patch

What happens here is that when rebuilding text runs after bidi resolution BuildTextRunsScanner::ContinueTextRunAcrossFrames splits the text runs between the Latin text frame and the Arabic text frame (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrameThebes.cpp?mark=1316-1325#1299). Later, however, the frame with the Arabic text is removed (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsLineLayout.cpp?mark=1001-1011#985), and the previous frame is left with offsets pointing beyond the end of its text run.

I'm not sure if this is the ideal fix: it might be better to prevent the previous frame from being marked as NS_FRAME_COMPLETE, but I'm not quite sure where that happens.
Attachment #375189 - Flags: superreview?(roc)
Attachment #375189 - Flags: review?(roc)
(Assignee)

Comment 4

9 years ago
(The patch fixes bug 489691 also, so I have added crashtests for both bugs)
Blocks: 489691
Comment on attachment 375189 [details] [diff] [review]
Patch

+    if (mPrevContinuation) {
+      nsTextFrame* textFrame = static_cast<nsTextFrame*>(mPrevContinuation);
+      if (textFrame) {
+        textFrame->ClearTextRun();

You don't have to recheck textFrame for null here.
Attachment #375189 - Flags: superreview?(roc)
Attachment #375189 - Flags: superreview+
Attachment #375189 - Flags: review?(roc)
Attachment #375189 - Flags: review+
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/051f635a1061
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
verified FIXED on debug builds (no crash or asserts):

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090528 Shiretoko/3.5pre ID:20090528130303

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090528 Minefield/3.6a1pre ID:20090528112613
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.