103036 - classic: crash when trying to open Preferences dialog

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect)

Description

[sairuh (rarely reading bugmail)]

found this while testing bug 102858, using 2001.10.03.08-trunk comm bits on Mac
OS 9.x.

this only occurs on Mac [don't have a trunk 10.x build from today, tho'], and
only with the classic theme. also occurred with both a new and existing profile.

1. launch profile that's using classic theme.
2. select Edit > Preferences.

result: crash.

i'll dig up the talkback data soon. not sure if this would be considered a dup
of 102858, but lemme know if it is.

tracy et al., have you encountered this?

btw, disable_xul_cache is the default value: false.

Comment 1

[sairuh (rarely reading bugmail)]

the talkback data are useless --all "unknown program exit" and empty traces.

let me try macsbug...

Comment 2

[Chase Tingley]

dup of bug 102858?

Comment 3

[sairuh (rarely reading bugmail)]

hrm, i'm having problems getting trace info from macsbug --whatever logfiles i
try to create aren't being saved at the moment. could anyone with a debug build
get more info on this, please?

but here's a really wacky observation: if i open the mail or addressbook window
*first* then try to open Preferences, i don't encounter a crash and the dialog
does appear. [i tried this method with a composer window, but still got the crash.]

Comment 4

[sairuh (rarely reading bugmail)]

as i mentioned earlier, i'm not sure if this is the same as bug 102858
[different platform, theme issue]. however, if folx here are sure that it is, go
ahead and dup it. :)

Comment 5

[Simon Fraser [no longer active]]

This one is crashing with a CHK Exception at _PBReadImmed, and hoses the machine.

Comment 6

[sairuh (rarely reading bugmail)]

marking this a smoketest blocker: if the machine is running 9.x [ie, *not*
classic emulation mode on os 10.x], i'm forced to reboot the box. ouch.

Comment 7

[sairuh (rarely reading bugmail)]

i believe this is trunk-only --this isn't a problem with 2001.10.03.03-branch
comm bits.

curious to see if this is also problem with mac os x bits...but will need to
wait till tomorrow for a more recent verif trunk build.

Comment 8

[sairuh (rarely reading bugmail)]

not a problem using 2001.10.03.04-branch comm bits on mac os 10.0.4. will check
os x trunk bits tomorrow.

Comment 9

[Simon Fraser [no longer active]]

I see this on Mac OS X as well. It just seg faults, no console output, and no
Crash Report.

Comment 10

[Kathleen :Brade]

in Macsbug, I'm landing in CodeWarrior.  :-/

Comment 11

[sairuh (rarely reading bugmail)]

Attachment: crash report from 2001.10.03.20-trunk on Mac OS 10.0.4

Comment 12

[Joe Hewitt (gone)]

Just tested on my mac, and I am crashing using the Modern theme too.

These problems are probably derived from my <dialog/> changes.  I migrated the
prefs dialog from using the old dialogOverlay.css to the new dialog tag. 
Everything works fine on Win and Linux, and I am at a loss for why it crashes on
Mac.

I also converted the Find dialog and File Bookmark dialog, which don't crash,
but seem to be not working right on Mac.  Again, all is well on Win and Linux.  

Could this have something to do with the fact that the root element of these
dialogs is no longer <window/>, but <dialog/> ? Hyatt and I scoured the tree for
dependencies on the window tag as the root element, and found only one (which I
fixed).  Maybe there is something we missed that is Mac specific?

Comment 13

[David Hyatt]

Joe, try applying aaronl's patch to delay the advanceFocus call until onload.

Comment 14

[sairuh (rarely reading bugmail)]

joe: are you crashing w/modern using a debug or verification build? i haven't
[yet] seen a crash on modern when just opening the Prefs dialog, but i'm just
using verif bits... however, kathy recently aim'd me saying that she's crashing
w/modern --perhaps that scenario is limited to debug bits [so far]?

aaronl's patch is in/for bug 58853, iirc.

Comment 15

[Jean-Francois Ducarroz]

ducarroz and I are testing our aarol's patch
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=51940) on the mac now.

Comment 16

[(not reading, please use [e-mail] instead)]

aarol's patch fixes this crasher (debug, mac 0S 9.2, classic)

Comment 17

[Simon Fraser [no longer active]]

So can anyone say exactly where it crashed, and why?

Comment 18

[Kathleen :Brade]

Based on the crash report from Sairuh,  I confirmed by stepping in the debbugger  
that we were recursively calling "FocusAvailable" so we were blowing out the 
stack.

Comment 19

[Kathleen :Brade]

assuming this fix has been tested on other OS, r=brade

Comment 20

[Simon Fraser [no longer active]]

I think we need to add some recursion protection there then; it should not be the 
case that bad XBL or XUL can cause a crash like this, and aaron's fix is XBL 
only, right?

Comment 21

[Simon Fraser [no longer active]]

Patch, anyone?

Comment 22

[Simon Fraser [no longer active]]

I don't understand how the last line of nsDocShell::FocusAvailable() is ever
supposed not to recurse indefinately. cc Adam Lock, who I hope can clarify.

Comment 23

[Samir Gehani]

*** Bug 102858 has been marked as a duplicate of this bug. ***

Comment 24

[Simon Fraser [no longer active]]

Spam other focus/docShell folks.

Comment 25

[Adam Lock]

The only way I can see it going crazy like this is if the docshell has no parent
and no children to switch focus to so that the code keeps dropping through to
that last line recursively. Is it possible that preference panes might somehow
trigger this kind of state in a docshell?

CC'ing bryner for ideas

Comment 26

[Simon Fraser [no longer active]]

Adam: how can that FocusAvailable() call at the end of 
nsDocShell::FocusAvailable() ever do anything different than when it was called 
the first time? How is it not the kiss of recursion death if that line is ever 
hit?

Comment 27

[Brian Ryner (not reading)]

Well, my upcoming patches will be removing FocusAvailable() entirely, so I'm at
least confident that we won't see this exact stack in a crash report :)

Seriously though, I'm not seeing this in my tree where I have all of my tabbing
changes, but that's on Linux.  I'll try to get someone to test on Mac.

Comment 28

[Adam Lock]

The answer is I have no idea :) I think this is Travis code if that offers some
kind of explanation.

Comment 29

[Adam Lock]

Perhaps a simple fix is to replace that line with NS_OK and see what happens. I
can't reproduce the problem so can someone try that and report back?

Comment 30

[Adam Lock]

Joe, a quick question about your dialog changes. Does your dialog class
implement ensure the docshell has a docshelltree owner and that it also
implements the nsIBaseWindow interface. If not then it's possible this is what
is causing the docshell to think it has no parent.

Comment 31

[Simon Fraser [no longer active]]

Returning NS_OK works fine; it's only ever hit in this pathalogical case of prefs 
dialog with classic skin and XBL badness.

Comment 32

[Adam Lock]

Bug 70750 contains Joe's dialog changes. 

I notice a small problem in the checkin, in that the "window" style is listed
twice in the Mac's global.css file when it should be "window" & "wizard" as it
is in the Win32 global.css. That couldn't be the cause of the problem could it?

http://lxr.mozilla.org/seamonkey/source/themes/classic/global/mac/global.css#103
http://lxr.mozilla.org/seamonkey/source/themes/classic/global/win/global.css#113

Comment 33

[Simon Fraser [no longer active]]

Attachment: Stack for the recursive FocusAvailable() call

Comment 34

[Simon Fraser [no longer active]]

FocusAvailable() is recursing because:
  i) FocusNextChild(aCurrentFocus) refuses to take focus because the tree item
     type is chrome, not content.
 ii) nextCallWin->FocusAvailable() calls nsXULWindow, which doesn't take focus.
iii) FocusNextChild(child) refuses to take focus because the tree item
     type is chrome, not content.

so it just recurses.

Comment 35

[Simon Fraser [no longer active]]

I should add that child->SetFocus() fails because the child has no pres shell 
yet.

Comment 36

[Adam Lock]

Behaviour is probably being triggered in the first place by some focus setting
code in the dialog constructor in dialog.xml

http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/dialog.xml#70

Comment 37

[Simon Fraser [no longer active]]

Attachment: Stopgap patch

Comment 38

[Simon Fraser [no longer active]]

Comment on attachment 52130 [details] [diff] [review]
Stopgap patch

Better patch coming

Comment 39

[Simon Fraser [no longer active]]

Attachment: Better patch; don't recurse with the same arguments

Comment 40

[Adam Lock]

2nd patch looks good, r=adamlock

I think there is a wider issue with setting focus from the dialog constructor to
be resolved and the window,window bug in the Mac's global.css.

Comment 41

[David Hyatt]

Aaronl's dialog.xml patch should also land with this.  I give sr on both patches.

Comment 42

[Simon Fraser [no longer active]]

I checked in 
http://bugzilla.mozilla.org/attachment.cgi?id=52132
and 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=51940

Comment 43

[Tracy Walker [:tracy]]

this has been working fine, most recently on mac 2001-25-25-04-trunk