Loading the testcase triggers an assertion that was added in bug 412093: ###!!! ASSERTION: Cannot find containing block.: 'NS_SUCCEEDED(rv) && mBlockFrame', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSRendering.cpp, line 267 followed by a crash [@ InlineBackgroundData::GetContinuousRect]. The testcase seems to depend on the image being loaded remotely; replacing the image URL with a data: URL makes the crash go away.

When we hit the assertion |frame| is not null, but isn't a block frame either. Rather, it's a nsDocElementBoxFrame. This could be easily patched up in a couple of ways (put this block in mBlockFrame even though it isn't one; just assume LTR if mBlockFrame is NULL), but I'm not sure either of these is right. I'm also not really sure where I got that code for finding the containing block. Roc - could you suggest a better way for finding the (block) element that establishes the base direction for bidi purposes - preferably one that always returns some result?

What does the frame tree look like in that bug? It sounds like it's a bad frame tree. Inline frames should always be wrapped in blocks somewhere.

Frame dump: Viewport(-1)@0xc5c0ac [view=0x17263210] {0,0,36600,22860} [state=00042020] [content=0x0] [sc=0xc5bfb0] pst=:-moz-viewport< RootBox(window)(-1)@0xc5c140 {0,0,36600,22860} [state=81c50000] [content=0x1d9d08b0] [sc=0xc5c1b4] pst=:-moz-canvas< DocElementBox(window)(-1)@0xc5c47c {0,0,36600,22860} [state=81a50000] [content=0x1d9d08b0] [sc=0xc5c2b0]< PopupSet(popupgroup)(-1)@0xc5c578 next=0xc5c8f8 {0,0,36600,0} [state=80c50000] [content=0x1d5dc560] [sc=0xc5c3c4]< Popup-list for PopupSet(popupgroup)(-1)@0xc5c578 < MenuPopup(tooltip)(-1)@0xc5c7cc [view=0x172e0ad0] {0,0,0,0} [state=80812502] [content=0x1721c5d0] [sc=0xc5c714]<> > > Placeholder(tooltip)(-1)@0xc5c8f8 {0,0,36600,0} outOfFlowFrame=MenuPopup(tooltip)(-1)@0xc5c7cc Inline(hbox)(0)@0xc5c9d0 {0,0,36600,22860} [content=0x1d584580] [sc=0xc5c634]<> > > >

Yeah, that frame tree is bogus, the inline should not be a child of the DocElementBox. Not your bug.

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).

tree.gif is gone :(

Same happens on Vista. Here the crash id: bp-f41d6b17-ad30-11dd-a2ae-001cc4e2bf68

I think the problem here is that bug 321402 didn't patch ConstructDocElementFrame, but it should have.

I think we should just move this into ProcessChildren. (The other potentially problematic caller is nsCSSFrameConstructor::LazyGenerateChildrenEvent::Run.)

This passes reftests, crashtests, chrome mochitests, and browser-chrome mochitests. This patch changes how we handle text children of the root element in XUL: we now start displaying them ,like we did for bug 321402 for non-root XUL elements. Oddly enough, the only tests depending on the broken behavior were three of the tests for bug 321402 itself, which had some broken markup in them.

> This patch changes how we handle text children of the root element in XUL: > we now start displaying them Please add a reftest reflecting this intentional behavior change :)

The tests that Jesse encouraged me to add exposed that another change was needed to prevent the tests from crashing; see the additional change to nsPopupSetFrame. Ran all reftests, chrome tests, and browser-chrome tests. Unfortunately, the act of adding the additional tests causes 446100-1[efgh].html to start breaking in my tree; they're showing yellow boxes. (If I comment out 421203-6, then only f and g break.) I'm a little suspicious of some weird side-effect of the canvas caching...

Comment on attachment 352852 [details] [diff] [review] patch I think this patch is OK; I'll continue to debug the reftest issue. (Disabling the canvas caching does fix it. Next thing to test will be to see if copying the references so we don't increase the canvas cache count across those tests also fixes it.)

zwol also hit problems with those reftests in bug 454349 comment 25.

Comment on attachment 352852 [details] [diff] [review] patch I'd prefer to see that "find the root box" code in nsLayoutUtils or possibly nsPresShell or nsFrameManager.

Are there other places that do "find the root box"? That code is also "find the root box from a popupset"...

Yes there are... turns out they call nsIRootBox::GetRootBox, so let's call that here too :-).

Calls nsIRootBox::GetRootBox.

I think we need to make that conditional on rootBox not being null. IIRC you can restyle a XUL <window> root element to display:table and end up with a document that has a popupset but no root box.

As requested, and also changes nsPopupSetFrame::Init to match.

Landed on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/3555a8de08b2 http://hg.mozilla.org/mozilla-central/rev/4a73efb88edc ... along with bug 454349.

Had to land one followup fix to a test as well: http://hg.mozilla.org/mozilla-central/rev/135f996f1c6c

I don't think we can land this on 1.9.1 given what we've said about extension compatibility.

(In reply to comment #26) > I don't think we can land this on 1.9.1 given what we've said about extension > compatibility. Yeah, looks like it breaks Speed Dial 0.7.2.7, for one. Also, some other possibly related discussion here: http://forums.mozillazine.org/viewtopic.php?p=5258105&sid=742a848b5fc8306b55332a716358951d#p5258105

(In reply to comment #26) > I don't think we can land this on 1.9.1 given what we've said about extension > compatibility. What exactly would cause extensions to break and what would extensions authors need to do to fix things? Attached are screenshots of Chatzilla and Auto Dial; and both have some degree of oddness. Chatzilla is totally unusable with layout all messed up. Auto Dial isn't too bad, but each row has equalsize=always with items flex=1, but it seems that if the content doesn't need to be cropped, flex doesn't kick in (difference in gray background color). And just a sanity check, reverting to 4a73efb88edc shows the problem but 5bf3661db065 is fine.

Extension authors would need to look at the error console (which will have a message describing why the root element's contents were wrapped in a block) and make the offending item described there 'display:none', remove it, or comment it out, depending on what was desired. Chatzilla trunk is already fixed; see bug 469983.

Here's a branch wallpaper patch. The test is the same as trunk. I propose landing this on the 1.9.1 branch only.

I should probably explicitly set mBlockFrame = nsnull as well.

Landed wallpaper on 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c37466180679

David, can you please enlighten me for verifying this bug? When I use the self-contained testcase (attachment 338407 [details]) Minefield is presenting a blank page while Shiretoko displays our tree image repeated in x and y direction. I don't think that this is expected, or?

It seems to me that Minefield is showing the correct behavior. When an elements is display: inline, it doesn't seem to me that flex should make any difference anymore. Also, the width of the element becomes 0 on trunk, which is what I also would expect for an empty inline element. Verified fixed (for not crashing anymore, at least): Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090208 Shiretoko/3.1b3pre (.NET CLR 3.5.30729)

I noticed this also crashes on 1.9.0.x, is the branch wallpaper fix also suitable for 1.9.0.x?

(In reply to comment #34) > It seems to me that Minefield is showing the correct behavior. When an elements > is display: inline, it doesn't seem to me that flex should make any difference > anymore. Also, the width of the element becomes 0 on trunk, which is what I > also would expect for an empty inline element. So that means Shiretoko doesn't handle it correctly? Shall we file a new bug on that? Martijn, could you do that? You have more insight what should going on here.

We should take this in 1.9.0 too

Comment on attachment 360787 [details] [diff] [review] branch wallpaper [Checkin: Comment 32 & 39] Approved for 1.9.0.8, a=dveditz for release-drivers

Checked in to CVS trunk for 1.9.0.8, 2009-03-08 12:15 -0700.

Verified for 1.9.0.8 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.8pre) Gecko/2009031904 GranParadiso/3.0.8pre. Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090319 Shiretoko/3.5b4pre.