See testcase, this crashes on load for me when loading, with current trunk builds. This regressed between 2007-01-12 and 2007-01-13: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-12+04&maxdate=2007-01-13+07&cvsroot=%2Fcvsroot I think this is a regression from bug 366207.

WFM on Mac (trunk nightly 2007-01-22) with Java enabled. Is the XUL mime type necessary, or does it crash with an XHTML mime type too?

Same thing happens if I load the testcase as XHTML.

So here's why we crash, as far as I can tell: 1) We go to create frames for the outer <object>. This constructs a block frame for it (not an nsObjectFrame), because it's the root element. 2) Since overflow is not set hidden, we construct the root scrollframe. 3) We create the anonymous <scrollbar> nodes. 4) Creating frames for the <scrollbar>s attaches their XBL bindings. 5) Binding attachment calls WrapNative on the <scrollbar>s. 6) Being XUL, the wrapper parent is whatever GetParent() returns, which in this case is the <object>. 7) Wrapping the <object> triggers EnsureInstantiation (from classinfo). 8) EnsureInstantiation tries to reconstruct the frame for the <object>; we reenter frame construction and start doing stuff, but the frame tree is in a bogus state right then, and we crash. This patch changes step 6. I don't know why XUL parents to the GetParent() but I assume it's to force binding instantiation on ancestors if a child is wrapped. But in this case, for anonymous content, I don't really think we want to force such binding instantiation... I suppose I could also just compare the GetBindingParent() of the kid with that of the parent. That would have the same result and might be clearer.

There are other reasons for parenting to the parent in XUL I think. It'll mean that things like event-handler-attributes see properties of the entire parent chain when they execute. The two scary steps in the above description sounds to me like 5 and 7. Especially, could we delay the EnsureInstantiation call until later?

> things like event-handler-attributes see properties of the entire parent > chain But do we want that for anonymous kids? I agree that step 7 is scary. The issue is that we assume that being wrapped means script is asking for us, and if it is we have to assume it might want to call functions on the plugin... so we have to instantiate so we can install the plugin prototype stuff, etc. The problem is that we could be all ready to instantiate but not have done it yet, because normally instantiation happens off an event. Now the other possible fix is to do step 4 later. Or rather, split up binding attachment into two separate parts -- creation of anonymous content (which we do want to happen during frame construction) and attachment of the JS gunk (which is what's biting us here, and which we could try doing at a "safe" time, e.g. right before calling the constructor). A third option is to only parent XUL to the parent node if the parent node is also XUL. ;)

Hmm.. why do scrollbars have the JS gunk at all? Is this due to the mac stuff?

I think so... though the handlers might need it too; I can't recall.

I have ideas for getting rid of the mac script stuff. Maybe we could do that and see if it solves this?

We could try, sure.

i think the main purpose was supporting scrollbars at either end. note of course that beos still allows for this, and theoretically some other system themes on linux might :).

Comment on attachment 252884 [details] [diff] [review] Possible patch Clearing sr flag until Jonas figures out whether we need this or not.

Comment on attachment 252884 [details] [diff] [review] Possible patch I think I'll take Jonas' earlier comment as r-.

Patch in bug 384612 should have fixed this.

verified fixed using the testcase and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010104 Minefield/3.0b3pre ID:2008010104. No crash on testcase -> Verified fixed