I don't think that the nsBoxToBlockAdaptor should be creating a space manager. Instead, I think what we probably ought to do is make sure that blocks that are contained within box frames are marked with the NS_BLOCK_SPACE_MGR bit so that the block frame can create and own the space manager.

Presumably this testcase would break (probably crash) if there were no space manager.

This ought to do the trick, although I'm not sure it's the cleanest way to do it. Is there a better place to be setting the NS_BLOCK_SPACE_MGR flag? Also, we wrap a fair number of non-block frames with the nsBoxToBlockAdaptor: on startup, I get... *** nsBoxToBlockAdaptor: wrapping non-block frame Canvas(html)(-1)@0x824db08 *** nsBoxToBlockAdaptor: wrapping non-block frame Placeholder(tooltip)(-1)@0x84ddb70 *** nsBoxToBlockAdaptor: wrapping non-block frame Placeholder(tooltip)(0)@0x850efec *** nsBoxToBlockAdaptor: wrapping non-block frame Placeholder(tooltip)(0)@0x85af3b8 *** nsBoxToBlockAdaptor: wrapping non-block frame FrameOuter(browser)(0)@0x85e8c10 *** nsBoxToBlockAdaptor: wrapping non-block frame Canvas(html)(-1)@0x860dc78 *** nsBoxToBlockAdaptor: wrapping non-block frame Placeholder(popup)(0)@0x865cdec *** nsBoxToBlockAdaptor: wrapping non-block frame LeafBox(outlinerbody)(0)@0x8679ae0 *** nsBoxToBlockAdaptor: wrapping non-block frame Canvas(html)(-1)@0x8758e14 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(0)@0x8616c98 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(2)@0x86171d4 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(0)@0x86167f8 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(2)@0x8616a64 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(4)@0x8617350 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(6)@0x8617550 *** nsBoxToBlockAdaptor: wrapping non-block frame Placeholder(tooltip)(-1)@0x861aeb8 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(0)@0x861af4c *** nsBoxToBlockAdaptor: wrapping non-block frame Text(2)@0x86165e8 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(4)@0x860dcb8 *** nsBoxToBlockAdaptor: wrapping non-block frame Text(6)@0x860deb8 *** nsBoxToBlockAdaptor: wrapping non-block frame FrameOuter(browser)(0)@0x8830324 *** nsBoxToBlockAdaptor: wrapping non-block frame Placeholder(tooltip)(-1)@0x87d37bc *** nsBoxToBlockAdaptor: wrapping non-block frame Canvas(html)(-1)@0x8765ba8 *** nsBoxToBlockAdaptor: wrapping non-block frame Canvas(html)(-1)@0x8859af0 *** nsBoxToBlockAdaptor: wrapping non-block frame Canvas(html)(-1)@0x88418b4 There aren't that many of these, so it's probably not too wasteful, but I was thrown off a bit. Was this really what evaughan expected?

How about at least removing the member variable? What about patching the frame construction code instead -- or is that hard?

Oops, I did remove the member variable but didn't include the header file when I created the patch. And I'll take another look at the frame construction code.

Okay, I dug a little bit deeper. It turns out that some <xul:label> and <xul:description> elements are creating block frames by virtue of the fact that they've got a `display: block' in xul.css (the XUL label frame creation code is tortured, as jag can attest to after we dug through it last week). If we want these to be blocks, then we'll need some way of calling these frames out for the frame constructor so that it can create area frames with the NS_BLOCK_SPACE_MGR (and probably NS_BLOCK_MARGIN_ROOT) bit(s) set. I could create another CSS property (or another value for the CSS `display') that would cause this to happen. Or, I could simply force the XUL <label> and <description> to create area frames, and remove the style rule from xul.css. I'm inclined to do the latter, since I don't see any reason that people would want to change the display type for these elements. Once I fix this problem, it should be possible to remove the space manager code from box-to-block adapter completely. hyatt, jag, dbaron: do you have any thoughts?

(FWIW, I was able to verify that this approach will work. Specifically, I removed the space manager code for nsBoxToBlockAdaptor.cpp, and then added `overflow: scroll' to the rule for <label> and <description> in xul.css: this forces us to create an area frame with the NS_BLOCK_SPACE_MGR bit set.)

Remove space-manager cruft from nsBoxToBlockAdaptor; hard-code <xul:description> and <xul:label> to use area frames.

Comment on attachment 66221 [details] [diff] [review] proposed fix r=dbaron, although I wonder whether you want NS_NewBlockFrame or NS_NewAreaFrame.

Comment on attachment 66221 [details] [diff] [review] proposed fix r=dbaron, although I wonder whether you want NS_NewBlockFrame or NS_NewAreaFrame.

Comment on attachment 66221 [details] [diff] [review] proposed fix sr=hyatt

Fix checked in.

can this have caused bug 121860?