Crash with arabic character, line separator character, and missing-glyph

RESOLVED WORKSFORME

Status

()

Core
Graphics
--
critical
RESOLVED WORKSFORME
11 years ago
10 years ago

People

(Reporter: Jesse Ruderman, Assigned: roc)

Tracking

(Blocks: 1 bug, {assertion, crash, testcase})

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Created attachment 261327 [details]
testcase (crashes Firefox when loaded)

Loading this testcase triggers an assertion and a crash.


###!!! ASSERTION: Couldn't find glyph for trailing marker: 'glyphRecords[0].originalOffset == aRun->GetLength()*2', file /Users/jruderman/trunk/mozilla/gfx/thebes/src/gfxAtsuiFonts.cpp, line 680


Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
0   libthebes.dylib          	0x07b37281 nsAutoArrayPtr<gfxTextRun::DetailedGlyph>::get() const + 9 (nsAutoPtr.h:575)
1   libthebes.dylib          	0x07b37297 nsAutoArrayPtr<gfxTextRun::DetailedGlyph>::operator gfxTextRun::DetailedGlyph*() const + 17 (nsAutoPtr.h:589)
2   libthebes.dylib          	0x07ad4393 gfxTextRun::GetAdvanceWidth(unsigned, unsigned, gfxTextRun::PropertyProvider*) + 953 (gfxFont.cpp:1350)
3   libgkgfxthebes.dylib     	0x30d11702 nsThebesFontMetrics::GetWidth(unsigned short const*, unsigned, int&, int*, nsThebesRenderingContext*) + 252 (nsThebesFontMetrics.cpp:352)
4   libgkgfxthebes.dylib     	0x30d0f057 nsThebesRenderingContext::GetWidthInternal(unsigned short const*, unsigned, int&, int*) + 93 (nsThebesRenderingContext.cpp:1100)
5   libgkgfxthebes.dylib     	0x30d12db8 nsRenderingContextImpl::GetWidth(unsigned short const*, unsigned, int&, int*) + 122 (nsRenderingContextImpl.cpp:524)
6   libgkgfxthebes.dylib     	0x30d1aa4a nsThebesRenderingContext::GetWidth(unsigned short const*, unsigned, int&, int*) + 48 (nsThebesRenderingContext.h:153)
7   libgkgfxthebes.dylib     	0x30d0ef7a nsThebesRenderingContext::GetTextDimensionsInternal(unsigned short const*, unsigned, nsTextDimensions&, int*) + 130 (nsThebesRenderingContext.cpp:1121)
8   libgkgfxthebes.dylib     	0x30d12f66 nsRenderingContextImpl::GetTextDimensions(unsigned short const*, unsigned, nsTextDimensions&, int*) + 74 (nsRenderingContextImpl.cpp:570)
9   libgklayout.dylib        	0x197598cc nsTextFrame::MeasureText(nsPresContext*, nsHTMLReflowState const&, nsTextTransformer&, nsTextStyle&, nsTextFrame::TextReflowData&) + 2394 (nsTextFrame.cpp:5334)
10  libgklayout.dylib        	0x1975d326 nsTextFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 1546 (nsTextFrame.cpp:6114)
11  libgklayout.dylib        	0x197267a1 nsLineLayout::ReflowFrame(nsIFrame*, unsigned&, nsHTMLReflowMetrics*, int&) + 1165 (nsLineLayout.cpp:888)
12  libgklayout.dylib        	0x1971c865 nsInlineFrame::ReflowInlineFrame(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsIFrame*, unsigned&) + 75 (nsInlineFrame.cpp:546)
13  libgklayout.dylib        	0x1971d585 nsInlineFrame::ReflowFrames(nsPresContext*, nsHTMLReflowState const&, nsInlineFrame::InlineReflowState&, nsHTMLReflowMetrics&, unsigned&) + 529 (nsInlineFrame.cpp:413)
14  libgklayout.dylib        	0x1971e151 nsInlineFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 839 (nsInlineFrame.cpp:345)
15  libgklayout.dylib        	0x197267a1 nsLineLayout::ReflowFrame(nsIFrame*, unsigned&, nsHTMLReflowMetrics*, int&) + 1165 (nsLineLayout.cpp:888)
16  libgklayout.dylib        	0x196cd071 nsBlockFrame::ReflowInlineFrame(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) + 153 (nsBlockFrame.cpp:3427)
17  libgklayout.dylib        	0x196d4b12 nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, int*, LineReflowStatus*, int) + 692 (nsBlockFrame.cpp:3247)
18  libgklayout.dylib        	0x196d5284 nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, int*) + 296 (nsBlockFrame.cpp:3095)
19  libgklayout.dylib        	0x196d5840 nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, int*) + 1032 (nsBlockFrame.cpp:2176)
20  libgklayout.dylib        	0x196d5f63 nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) + 1671 (nsBlockFrame.cpp:1787)
21  libgklayout.dylib        	0x196d7f19 nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned&) + 857 (nsBlockFrame.cpp:911)
This is happening because there's a Unicode line separator in the string. The string passed to ATSUI is "LSEP ." and it's RTL. ATSUI returns a (pseudo?) glyph for the LSEP followed by a glyph for the period --- I guess because it's giving us the first line and then the second line. This confuses gfxAtsuiFontGroup. We should probably strip LSEP somehow.
Created attachment 261335 [details] [diff] [review]
fix

Fix the bug by converting LSEP and PSEP to zero. ATSUI converts codepoint zero to an empty zero-width glyph so this works fine. If we need to handle LSEP and PSEP in some other way, they should probably be handled by the new textframe.
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #261335 - Flags: review?(vladimir)
(Reporter)

Updated

11 years ago
Blocks: 377438
(Reporter)

Comment 3

11 years ago
See also bug 377461, "Crash with \r and some arabic characters".
Component: Layout: Fonts and Text → GFX: Thebes
Created attachment 261530 [details] [diff] [review]
fix the crash in bug 377461 as well

Check for \r and \n as well.

I also changed from using an nsAutoString for the temporary string to using an nsAutoTArray, because in this case the latter is just as simple and probably more efficient.
Attachment #261335 - Attachment is obsolete: true
Attachment #261530 - Flags: review?(vladimir)
Attachment #261335 - Flags: review?(vladimir)
(Reporter)

Updated

11 years ago
Blocks: 377461
QA Contact: layout.fonts-and-text → thebes
(Reporter)

Comment 5

11 years ago
WFM on trunk.
Is this patch still needed?  Sounds like the testcase works for Jesse.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 7

10 years ago
Crashtest checked in.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.