387564 - sm crashes when setting message direction after changing charset

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P4)

Description

[Eyal Rozenberg]

Attachment: extension triggering the crash

When viewing some messages in the soon-to-be-attached folder with the BiDi Mail UI extension enabled, there are:

SM trunk nightly    ...
----------------------------
2007-07-01          no problems
2007-07-02          hang
2007-07-03          hang
2007-07-04          crash
...
2007-07-10          crash

Robert Kaiser suggests the culprit might be the following checkin:

2007-07-01 12:25	timeless%mozdev.org 	mozilla/js/jsd/jsd_xpc.cpp 	1.82 	25/18  	Bug 385765 [@ jsdASObserver::Observe] You can't dereference a NULL nsCOMPtr with operator->()
r=dveditz sr=dveditz

Comment 1

[Eyal Rozenberg]

Attachment: mbox file with which crash is triggered

The bug is triggered when the (single, Hebrew) message is selected.

Comment 2

[Robert Kaiser]

Eyal said that http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=TB33906923Z was the crash and that it regressed from -07-01 to -07-02, which led to this suspicion...

Comment 3

[timeless]

um, it should be easy to disable jsd (disable venkman, disable firebug, and then remove the jsd startup observer). if you still see the crash/hang, it isn't my fault.

please catch this in a debugger and grab me on irc.

Comment 4

[Eyal Rozenberg]

(In reply to comment #3)
> disable venkman, disable firebug,

as extensions you mean?

> and then remove the jsd startup observer

How?

Comment 5

[Eyal Rozenberg]

(In reply to comment #2)
> Eyal said that
http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=TB33906923Z
> was the crash

No, I think this wasn't the crash. These crashes happen without talkback being triggered. This one is another crash IIRC.

Comment 6

[timeless]

> as extensions you mean?

yes

> and then remove the jsd startup observer

open the right compreg.dat and remove:
xpcom-autoregistration,JSDebugger Startup Observer,@mozilla.org/js/jsd/app-start-observer;2
app-startup,JSDebugger Startup Observer,service,@mozilla.org/js/jsd/app-start-observer;2

If those lines aren't there, then it isn't triggering itself on startup...

Comment 7

[Eyal Rozenberg]

Have done as you suggested; bug still manifests with 2007-07-10 nightly (a crash).

Comment 8

[Eyal Rozenberg]

Attachment: reduced triggering mbox file

Comment 9

[Eyal Rozenberg]

Attachment: reduced extension triggering crash/hang

The weird thing is, if I disable charset detection with this extension, it still triggers the crash, but if I remove the charset detection code altogether - it doesn't.

Comment 10

[Eyal Rozenberg]

Attachment: further reduced attachment with alert's

Now this is much more informative. The crash happens on the second call of the onload handler, after the charset is changed from "" (BTW, the fact that the charset has this value another bug, whose number I forget) to windows-1255. It is triggered by the attempt to set the direction style to rtl:

messageContentElement.style.direction = "rtl";

Comment 11

[Eyal Rozenberg]

Oh, small mistake: the reduced extension causes a hang, not a crash, with sm 2007-07-10 nightly.

Comment 12

[Eyal Rozenberg]

With a debug build, pulled from cvs with MOZ_CO_DATE = 2007-07-11 00:00:00, instead of a crash or a hang, I get the following to assertions in infinite loop:

Assertion: redo line on totally empty line: 'aState.IsImpactedByFloat()', file ... mozilla/layout/generic/nsBlockFrame.cpp, line 3353

Assertion: unconstrained height on totally empty line: 'NS_UNCONSTRAINEDSIZE != aState.mAvilSpaceRect.height' file .../mozilla/layout/generic/nsBlockFrame.cpp, line 3355

I now have new suspects: July 1st checkins for:

2007-07-01 22:26	sharparrow1%yahoo.com 	mozilla/layout/generic/nsBlockReflowState.cpp 	3.528 	2/0  	Bug 386147: Don't cut off floats at the bottom of the page (part 1). r+sr=roc
2007-07-01 22:19	sharparrow1%yahoo.com 	mozilla/layout/generic/nsBlockReflowState.cpp 	3.527 	2/0  	Bug 343595: {inc} overflow:hidden float in overflow:hidden div doesn't get repositioned correctly. r+sr=roc
2007-07-01 18:20	roc+%cs.cmu.edu 	mozilla/layout/generic/nsTextFrameThebes.cpp 	3.47 	48/21  	Bug 386122. Allow text runs to end at preformatted newlines. Speeds up loading of plain-text files a lot because we don't have to have a textrun for the entire file. r=smontagu
2007-07-01 18:15	roc+%cs.cmu.edu 	mozilla/layout/reftests/text/reftest.list 	1.2 	3/0  	Bug 9101. Soft hyphen tests.
2007-07-01 18:14	roc+%cs.cmu.edu 	mozilla/layout/generic/nsTextFrameThebes.cpp 	3.46 	52/30  	Bug 9101. Fix various soft-hyphen bugs, primarily related to detecting when hyphenation is needed during a second-pass break-forcing line reflow. r=smontagu 

Comment 13

[Eli Friedman]

It's quite unlikely to be my checkins; it doesn't look like there are any floats involved.

Comment 14

[Eyal Rozenberg]

Attachment: stack trace

finally, a stack trace

Comment 15

[Robert Kaiser]

(In reply to comment #13)
> It's quite unlikely to be my checkins; it doesn't look like there are any
> floats involved.

Umm, are you sure? The stack trace has aState.IsImpactedByFloat() stuff at the top...

Comment 16

[Eyal Rozenberg]

The crash/hang happens with the patches upto 19:00 checked in, doesn't happen
with the patch upto 17:00. Ok, new prime suspect: bug 386122.

Comment 17

[Eli Friedman]

(In reply to comment #15)
> (In reply to comment #13)
> > It's quite unlikely to be my checkins; it doesn't look like there are any
> > floats involved.
> 
> Umm, are you sure? The stack trace has aState.IsImpactedByFloat() stuff at the
> top...

That's irrelevant; the assertion is firing because something that shouldn't happen without floats is in fact happening without floats.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Maybe related to bug 389428?

Comment 19

[Eyal Rozenberg]

Probably not, as the checkin which triggers this does not involve gfxTextRun::CompressedGlyph::IsClusterStart AFAICT.

Comment 20

[Simon Montagu :smontagu]

Is this still happening? I didn't manage to reproduce with current trunk

Comment 21

[Eyal Rozenberg]

Still seeing it with sm trunk nightly from 2007-10-19.

Comment 22

[Eyal Rozenberg]

Seems to WFM with sm trunk nightly from 2007-12-06.

Comment 23

[Simon Montagu :smontagu]

Neither I nor the reporter can reproduce in current builds.