Closed
Bug 210647
Opened 21 years ago
Closed 21 years ago
[FIX] Crash when trying to change charsets to "user defined"
Categories
(Core Graveyard :: GFX: Gtk, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: pratik.solanki, Assigned: MatsPalmgren_bugz)
References
()
Details
(Keywords: crash, platform-parity, regression, Whiteboard: [patch])
Attachments
(4 files, 1 obsolete file)
1.11 KB,
text/plain
|
Details | |
7.43 KB,
text/plain
|
Details | |
1.53 KB,
patch
|
alecf
:
review+
|
Details | Diff | Splinter Review |
1.08 KB,
patch
|
dbaron
:
superreview+
brendan
:
approval1.5b+
|
Details | Diff | Splinter Review |
My steps to reproduce. 1. Create a fresh profile. 2. Add prefs for freetype font support. 3. Start Firebird 4. Change Serif and San-Serif font in Western and User Defined charsets to MS Verdana in your preferences. 5. Now use View | Character Coding to change back and fro from Wester to User defined. n the 3rd or 4th try, it crashes. Talkback ID - TB21348365M It also happens in Mozilla. I'll attach my copy of prefs.js
I should mention that this started happening with yesterday's (24 June) Firebird nightly and with my local Mozilla build dated 2003062405
From what I hear, Firebird talkback doesn't seem to have symbols. So here's a talkback for Mozilla TB 21377778H. Changing the product component appropriately.
Component: General → Layout
Product: Phoenix → Browser
Version: unspecified → Trunk
Comment 4•21 years ago
|
||
reassign
Assignee: blaker → other
Severity: major → critical
Keywords: crash,
stackwanted
QA Contact: asa → ian
Whiteboard: TB21377778H
->Gfx:Gtk. Incident ID 21377778 Stack Signature _Z19GetNormalLineHeightP14nsIFontMetrics() 81ce01f6 Email Address psolanki@netscape.net Product ID MozillaTrunk Build ID 2003062605 Trigger Time 2003-06-26 07:44:49 Platform LinuxIntel Operating System Linux 2.4.18-timesys-4.0 Module libgklayout.so URL visited User Comments Add this to bug 210647 Trigger Reason SIGSEGV: Segmentation Fault: (signal 11) Source File Name Trigger Line No. Stack Trace _Z19GetNormalLineHeightP14nsIFontMetrics() _Z17ComputeLineHeightP14nsIPresContextP19nsIRenderingContextP14nsStyleContext() _ZN17nsHTMLReflowState14CalcLineHeightEP14nsIPresContextP19nsIRenderingContextP8nsIFrame() _ZN18nsBlockReflowStateC1ERK17nsHTMLReflowStateP14nsIPresContextP12nsBlockFrameRK19nsHTMLReflowMetricsi() _ZN12nsBlockFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj() _ZN16nsContainerFrame11ReflowChildEP8nsIFrameP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateiijRj() _ZN11CanvasFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj() _ZN19nsBoxToBlockAdaptor6ReflowER16nsBoxLayoutStateP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRjiiiii() _ZN19nsBoxToBlockAdaptor8DoLayoutER16nsBoxLayoutState() _ZN5nsBox6LayoutER16nsBoxLayoutState() _ZN16nsScrollBoxFrame8DoLayoutER16nsBoxLayoutState() _ZN5nsBox6LayoutER16nsBoxLayoutState() _ZN14nsContainerBox13LayoutChildAtER16nsBoxLayoutStateP6nsIBoxRK6nsRect() _ZN21nsGfxScrollFrameInner9LayoutBoxER16nsBoxLayoutStateP6nsIBoxRK6nsRect() _ZN21nsGfxScrollFrameInner6LayoutER16nsBoxLayoutState() _ZN16nsGfxScrollFrame8DoLayoutER16nsBoxLayoutState() _ZN5nsBox6LayoutER16nsBoxLayoutState() _ZN10nsBoxFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj() _ZN16nsGfxScrollFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj() _ZN16nsContainerFrame11ReflowChildEP8nsIFrameP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateiijRj() _ZN13ViewportFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj() _ZN17IncrementalReflow8DispatchEP14nsIPresContextR19nsHTMLReflowMetricsRK6nsSizeR19nsIRenderingContext() _ZN9PresShell21ProcessReflowCommandsEi() _ZN11ReflowEvent11HandleEventEv() _Z13HandlePLEventP11ReflowEvent() PL_HandleEvent() PL_ProcessPendingEvents() _ZN16nsEventQueueImpl20ProcessPendingEventsEv() _Z24event_processor_callbackPvi17GdkInputCondition() _Z17our_gdk_io_invokeP11_GIOChannel12GIOConditionPv() libglib-1.2.so.0 + 0xea7a (0x4026da7a) libglib-1.2.so.0 + 0x10055 (0x4026f055) libglib-1.2.so.0 + 0x10659 (0x4026f659) libglib-1.2.so.0 + 0x107e8 (0x4026f7e8) libgtk-1.2.so.0 + 0x91203 (0x4018c203) _ZN10nsAppShell3RunEv() _ZN17nsAppShellService3RunEv() _Z5main1iPPcP11nsISupports() main()
Comment 6•21 years ago
|
||
does ... Stack Trace _Z19GetNormalLineHeightP14nsIFontMetrics() ... mean this is the code where the crash happens: http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsHTMLReflowState.cpp#2276 2276 GetNormalLineHeight(nsIFontMetrics* aFontMetrics) 2277 { 2278 NS_PRECONDITION(nsnull != aFontMetrics, "no font metrics"); 2279 2280 nscoord normalLineHeight; 2281 2282 #ifdef FONT_LEADING_APIS_V2 2283 nscoord externalLeading, internalLeading, emHeight; 2284 aFontMetrics->GetExternalLeading(externalLeading); 2285 aFontMetrics->GetInternalLeading(internalLeading); 2286 aFontMetrics->GetEmHeight(emHeight); 2287 switch (GetNormalLineHeightCalcControl()) { 2288 case eIncludeExternalLeading: 2289 normalLineHeight = emHeight+ internalLeading + externalLeading; 2290 break; 2291 case eCompensateLeading: 2292 if (!internalLeading && !externalLeading) 2293 normalLineHeight = NSToCoordRound(emHeight * NORMAL_LINE_HEIGHT_FACTOR); 2294 else 2295 normalLineHeight = emHeight+ internalLeading + externalLeading; 2296 break; 2297 default: 2298 //case eNoExternalLeading: 2299 normalLineHeight = emHeight + internalLeading; 2300 } 2301 #else 2302 aFontMetrics->GetNormalLineHeight(normalLineHeight); 2303 #endif // FONT_LEADING_APIS_V2 2304 return normalLineHeight; 2305 } If so, then does this imply that aFontMetrics is null?
Attached is the stacktrace I see under GDB. The problem I see is that aRenderingContext.GetFontMetrics(mNormalFont) returned with mNormalFont as NULL; thus crashing on the next line.
Looking up in my terminal I see the following intl.charset.detector = off intl.charset.detector = off Charset Overlay menu item pressed: x-user-defined WARNING: empty damage rect: update caller to avoid fcn call overhead, file /psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659 WARNING: empty damage rect: update caller to avoid fcn call overhead, file /psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659 WARNING: empty damage rect: update caller to avoid fcn call overhead, file /psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659 WARNING: empty damage rect: update caller to avoid fcn call overhead, file /psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659 ###!!! ASSERTION: font metrics should not be null - bug 136248: 'NS_SUCCEEDED(rv)', file /psolanki/mozilla/gfx/src/nsDeviceContext.cpp, line 706 Break: at file /psolanki/mozilla/gfx/src/nsDeviceContext.cpp, line 706 CCing rbs@maths.uq.edu.au who had fixed bug 136248.
> 2. Add prefs for freetype font support. [...] > Charset Overlay menu item pressed: x-user-defined Troubles arise if the user-defined charset/font is not handled well, see e.g., bug 147222. Te root cause here seems precisely that the user-defined charset/font is not handled well by the AA freetype support. My guess is that fm->Init() is failing in line 668 and line 687 in the code, just before the assertion above. Here is a handy way to try to see what is happening with your debugger. Add the following assertion on line 667: aMetrics = nsnull; nsresult rv = CreateFontMetricsInstance(&fm); if (NS_FAILED(rv)) return rv; rv = fm->Init(aFont, aLangGroup, mContext); + NS_ASSERTION(NS_SUCCEEDED(rv), "init failed"); and then break in the debugger from that point, and step inside the _next_ Init() [where the fontmetrics is given a second chance to redeem itself] rv = fm->Init(aFont, aLangGroup, mContext);
Reporter | ||
Comment 10•21 years ago
|
||
You're right. It does assert. In fact both init()s are failing. I can't seem to get Mozilla to run under debugger (although I can attach to it once it crashes) so I can't step through it like you suggested. I have the same font for Western and User Defined so I can't understand why it would crash. Does anyone else besides me see the crash? Is this reproducible or does it have something to do with my machine/font configuration?
Reporter | ||
Comment 11•21 years ago
|
||
*** Bug 213176 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 12•21 years ago
|
||
URL from dupe'd bug. Also here is a link to the testcase attached to that bug which causes Mozilla to crash. http://bugzilla.mozilla.org/attachment.cgi?id=128077&action=view P.S. Bugzilla should have a way of having multiple bugs point to the same attachment. Sort of like "Add existing attachment [id] to this bug"
Comment 13•21 years ago
|
||
*** Bug 216246 has been marked as a duplicate of this bug. ***
Comment 14•21 years ago
|
||
*** Bug 215478 has been marked as a duplicate of this bug. ***
Comment 15•21 years ago
|
||
this happens with and without freetype fonts. it's the user defined charset that causes the crash (see dupes, or just try changing to user defined without freetype fonts) this regressed between linux trunk 2003061105 and 2003061205, apparently bug 207438. I'll try backing it out.
Keywords: regression
Summary: Crash when trying to change charsets (with freetype fonts) → Crash when trying to change charsets to "user defined"
Comment 16•21 years ago
|
||
backing out bug 207438. the other checkin during that window to touch relevant files was bug 206379. In fact, that checkin modified the UserDefined section of nsFontMetricsGTK. http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/gfx/src/gtk&command=DIFF&root=/cvsroot&file=nsFontMetricsGTK.cpp&rev1=1.252&rev2=1.253#4 could that change be responsible for this behavior?
Comment 17•21 years ago
|
||
> backing out bug 207438. the other checkin... ugh. "backing out bug 207438 didn't help."
Comment 18•21 years ago
|
||
*** Bug 216444 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 19•21 years ago
|
||
The error was introduced between -r1.252 and -r1.253 of nsFontMetricsGTK.cpp as Andrew Schultz points out (with the bonsai link) in comment 16. We should not return prematurely if |gUserDefinedConverter| has already been initialized. This bit of code that the patch removes was previously the "else" part of the |GetCharsetAtom2| result test.
Assignee | ||
Updated•21 years ago
|
Keywords: pp
Summary: Crash when trying to change charsets to "user defined" → [FIX] Crash when trying to change charsets to "user defined"
Whiteboard: [patch]
Assignee | ||
Updated•21 years ago
|
Attachment #129935 -
Flags: review?(alecf)
Assignee | ||
Comment 20•21 years ago
|
||
Since this is a crasher that was caused by a "typo", it should be risc-free to include for 1.5b
Flags: blocking1.5b?
Flags: blocking1.5b? → blocking1.5b+
Comment on attachment 129935 [details] [diff] [review] Patch sr=dbaron if you convert the existing code to if (NS_FAILED(res)) return res; rather than having the returns in the |else|, so that this doesn't happen again.
Assignee | ||
Comment 22•21 years ago
|
||
Attachment #129935 -
Attachment is obsolete: true
Assignee | ||
Comment 23•21 years ago
|
||
Note that we have always ignored the return value from |SetOutputErrorBehavior|. Maybe we should comment that this is intentional? and even remove the assignment to |res|?
Assignee | ||
Updated•21 years ago
|
Attachment #129935 -
Flags: review?(alecf)
Assignee | ||
Updated•21 years ago
|
Attachment #129940 -
Flags: review?(alecf)
Attachment #129941 -
Flags: superreview+
Comment 24•21 years ago
|
||
Comment on attachment 129940 [details] [diff] [review] Patch rev. 2 r=alecf Generally this should go to i18n, though this particular fix is somewhat obvious...
Attachment #129940 -
Flags: review?(alecf) → review+
Comment 25•21 years ago
|
||
Comment on attachment 129941 [details] [diff] [review] Patch rev. 2 (diff -uw for review only) The patch misses the gfx/src/xlib part... ;-/
Attachment #129941 -
Flags: review-
Comment 26•21 years ago
|
||
Reassign to smontagu per IRC discussion, he said he takes care about the Xlib version of the patch...
Assignee: blizzard → smontagu
Comment 27•21 years ago
|
||
Back to Mats. I don't see that Xlib even *has* the bug :-)
Assignee: smontagu → mats.palmgren
Reporter | ||
Comment 28•21 years ago
|
||
So whats stopping this bug from being checked in? The patch has an r+ and an sr+ (Roland's r- doesn't seem to apply).
Attachment #129941 -
Flags: review- → approval1.5b?
Assignee | ||
Comment 29•21 years ago
|
||
Xlib and Windows does not have the bug. Mac does not have corresponding code AFAICT.
Comment 30•21 years ago
|
||
Comment on attachment 129941 [details] [diff] [review] Patch rev. 2 (diff -uw for review only) approved for 1.5b. /be
Attachment #129941 -
Flags: approval1.5b? → approval1.5b+
Comment 31•21 years ago
|
||
need to get this checked in today if its going to make 1.5b... trying to make builds thursday or friday morning. we could pick this up for final if it doesn't make the beta train.
Comment 32•21 years ago
|
||
looks like this would be a good one to get. showing as #5 talkback crash list for linux on the 1.4 release, but seems to only affect a small number of people...
Comment 33•21 years ago
|
||
Checked patch in (since Mats informed me that he doesn't have CVS access)
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 34•21 years ago
|
||
*** Bug 216980 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 35•21 years ago
|
||
Thanks Mats. I can confirm that the bug is fixed in the latest build - 2003082422.
Comment 36•21 years ago
|
||
*** Bug 226898 has been marked as a duplicate of this bug. ***
Updated•15 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•