433296 - "ASSERTION: Shouldn't happen" with listboxbody, iframe, tabpanels

Status: RESOLVED DUPLICATE of bug 467481

(Core :: XUL, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Shouldn't happen: 'aPresContext->GetPresShell()->GetPrimaryFrameFor(mContent) == this', file /Users/jruderman/trunk/mozilla/layout/generic/nsFrameFrame.cpp, line 518

This assertion was added in bug 396587.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This seems to workforme, though my build is as of MOZ_CO_DATE="Sat May 3 01:34:55 CDT 2008"...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Here's the problem part of the frame tree:

              Box(listboxbody)(0)@0x14dc4a4 [view=0xd93f1a0] {0,0,55200,3600} [state=80843400] [content=0xd9361b0] [sc=0x14f20a8] pst=:-moz-scrolled-content<
                Box(hbox)(1)@0x150ce34 next=0x14f236c {0,0,55200,960} [state=80c40010] [content=0xd489370] [sc=0x14f21c4]<
                  FrameOuter(iframe)(0)@0x150cfc0 [view=0xd950440] {0,0,0,960} [state=00002011] [content=0xd4893c0]
                >
                Box(listheader)(0)@0x14f236c next=0x150d7d4 {0,960,55200,960} [state=80400000] [content=0xd488e70] [sc=0x14f227c]<
                  ImageBox(image)(-1)@0x150caec {240,480,0,0} [content=0xd941700]
                  Area(label)(-1)@0x150cc88 next=0x150cdbc {360,480,54600,0} [state=00d00000] sc=0x150cbd0(i=0,b=0)<
                  >
                  ImageBox(image)(-1)@0x150cdbc {54960,480,0,0} [content=0xd941c10]
                >
                Box(hbox)(1)@0x150d7d4 {0,0,0,0} [state=80c40402] [content=0xd489370] [sc=0x14f21c4]<
                  FrameOuter(iframe)(0)@0x150d83c [view=0xd933060] {0,0,0,0} [state=00002402] [content=0xd4893c0]
                >
              >

Note that we have two boxes for the same content inside the listboxbody.  That would be a bug....

Comment 3

[Boris Zbarsky [:bzbarsky]]

And the real issue is that the listheader ended up _after_ the hbox.  That confuses the listboxbody code, which assumes that it needs to create a frame for the following sibling of the content of the listheader.

The reason _that_ happened is that xul.css has the following wonderful style rule:

443 listheader {
444   -moz-binding: url("chrome://global/content/bindings/listbox.xml#listheader");
445   -moz-box-ordinal-group: 2147483646;
446 }

so it ends up with a huge box-ordinal, and gets all mis-sorted.

Should listboxbody just be ignoring box-ordinals altogether, perhaps?

Comment 4

[[e-mail]]

(In reply to comment #3)
> 445   -moz-box-ordinal-group: 2147483646;
Surely that would happen with any element that had a -moz-box-ordinal-group style or an ordinal attribute?

> Should listboxbody just be ignoring box-ordinals altogether, perhaps?
Sounds good to me.

Note that  the ordinal only exists on the listhead because at one point we used to support tree splitters between listhead elements and they have it because treecol has it as part of treecol reordering support.

Comment 5

[Boris Zbarsky [:bzbarsky]]

>> Should listboxbody just be ignoring box-ordinals altogether, perhaps?
>Sounds good to me.

OK.  What are all the places that need changing?  It can't just be SetInitialChildList, right?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think listbox's dynamic creation and destruction of row frames is compatible with ordinals, so let's stop caring.

Upgrading to blocking since bug 467481 is blocking and basically requires the same fix.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

A little speleology shows the only code that checks ordinals is nsBoxFrame::CheckBoxOrder and nsBoxFrame::RelayoutChildAtOrdinal (which are the only callers of nsIFrame::GetOrdinal, which is the only caller of nsIBox::AddCSSOrdinal).

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I attached a patch to bug 467481.