68936 - trees in anonymous content crash mozilla

Status: RESOLVED WONTFIX

(Core :: XBL, defect)

Description

[Alex Fritze]

The binding:
<binding id="mywidget" extends="xul:box">
    <content>
        <xul:tree>
            <xul:treechildren>
                <xul:treerow>
                    <xul:treecell value="foo" flex="1"/>
                </xul:treerow>
            </xul:treechildren>
        </xul:tree>
    </content>
</binding>
crashes on load in nsXULTreeOuterGroupFrame::ComputeTotalRowCount(): 'parent' 
is =0 and parent->GetChildCount() fails.

Stack trace:

nsCOMPtr<nsIContent>::operator->() line 648 + 34 bytes
nsXULTreeOuterGroupFrame::ComputeTotalRowCount(int & 0x00000000, nsIContent * 
0x02d7b9f0) line 538 + 12 bytes
nsXULTreeOuterGroupFrame::GetRowCount() line 124 + 19 bytes
nsTreeLayout::GetPrefSize(nsTreeLayout * const 0x02d9a520, nsIBox * 0x02d7335c, 
nsBoxLayoutState & {...}, nsSize & {...}) line 98 + 8 bytes
nsContainerBox::GetPrefSize(nsContainerBox * const 0x02d7335c, nsBoxLayoutState 
& {...}, nsSize & {...}) line 445 + 38 bytes

Comment 1

[David Hyatt]

The tree is malformed.  You don't specify any columns.

Comment 2

[Alex Fritze]

This still crashes:

  <binding id="mywidget" extends="xul:box">
    <content>
        <xul:tree flex="1">
   	    <xul:treecolgroup>
	        <xul:treecol flex="1"/>
	    </xul:treecolgroup>
	    <xul:treehead>
                <xul:treerow>
                    <xul:treecell value="col" flex="1"/>
                </xul:treerow>		
	    </xul:treehead>
            <xul:treechildren flex="1">
		<xul:treeitem>
                <xul:treerow>
                    <xul:treecell value="foo" flex="1"/>
                </xul:treerow>
		</xul:treeitem>
            </xul:treechildren>
        </xul:tree>
    </content>
  </binding>

Comment 3

[David Hyatt]

Can I see the whole XML file, along with the XUL file you're using your widget in?

Comment 4

[Alex Fritze]

Attachment: testcase zip(treetest.xul/.css/.xml)

Comment 5

[Alex Fritze]

As far as I can see, the bug is in nsXULTreeOuterGroupFrame::GetTreeContent(), 
which tries to find the 'tree' content element given a 'treechildren' element. 
If the 'treechildren' element sits in a binding, only the binding parent is 
considered, not any possible intermediate parents of the treechildren element.

Comment 6

[Alex Fritze]

Attachment: proposed patch

Comment 7

[Alex Fritze]

Attachment: proposed patch

Comment 8

[Alex Fritze]

Attachment: better patch (makes scrollbars on trees work as well)

Comment 9

[Peter Trudelle]

Hyatt sez patch is incorrect, reverts code needed for making selects work based
off of trees. Don't know how to fix this. ->future

Comment 10

[Alex Fritze]

Is it? 
I'm probably missing something, but as far as I can see there are 3 situations 
in which behaviour is affected by the patch:

(1) 
   <mytree>: extends 'tree'
        <treechildren>: anonymous 
           ...
        
(2)
   <mytree>: extends 'tree'
        <some_tag>: anonymous
            <treechildren>: anonymous 
               ...

(3)
   <mywidget>
        <tree>: anonymous
            <treechildren>: anonymous 
                ...

In case (1), the patch is functionally equivalent to the old code since 
bindingparent == parent for nsXULTreeOuterFrame's content element (the 
<treechildren> element).

Case (2) should be illegal if <sometag> is not a tree, since in that case 
<treechildren> is not a direct child of a tree ?

Case (3) works with the patch but not without.

If case (2) is supposed to work, how about walking the parent chain instead of 
considering only the binding parent?

Comment 11

[David Hyatt]

I have deliberately broken your case in order to make trees able to use XBL to
do <select>s.  Your patch just removes the code that enables trees to do this.
A real patch will make both work.

Comment 12

[Alex Fritze]

Ah - I see. The 'select-size' binding puts a <treerows> element between <tree> 
and <treechildren>. So this is case (2).
I think all cases can be made to work by walking the parents until the <tree> 
is found.
Can't fully verify 'cause xbl <select>s are broken in my build.

Comment 13

[Alex Fritze]

Attachment: yet another attempt at a patch...

Comment 14

[David Hyatt]

Yeah, something broke them wihtin the last month or two.  I need to figure out
what happened.

Comment 15

[Alex Fritze]

I got some of them kind of working by deleting the 'includes="*"' attribs on 
<content> tags in platformSelectBindings.xml

Comment 16

[Alex Fritze]

Attachment: same patch updated for current source

I've been running with this patch locally for the past ~8 months and it seems
to works fine for me.

Comment 17

[Alex Fritze]

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

Comment 18

[Alex Fritze]

Attachment: updated for 0.9.8

Comment 19

[Joe Hewitt (gone)]

tree? we don't got no more stinking trees

Comment 20

[C. Daniel Mojoli B.]

I believe this bug or a related one is hitting us in development. We are using
Mozilla 1.0.1.

We are creating a tree with a template in the anonymous content. It *looks* like
this

<content>
  <xul:tree datasources="rdf:null" ref="urn:root">
    <xul:template>. . . </xul:template>

Then, through a method defined in the binding, we programmatically add a
datasource to the anonymous tree.

The tree does not display the content of the added datasource. We do not have a
problem with the rules of the template. We have tested this successfully quite a
lot outside of anonymous content.

In trying to figure out what is wrong, we did serveral things. Worth mentioning:
we tried issuing a rebuild on the tree builder after programmatically adding a
new datasource, but it crashed Mozilla without a stack trace nor assertion
notice on the console.

I would appreciate hints on this bug, or referral to other possibly preexisting
bugs.

Comment 21

[Brendan Eich [:brendan]]

Does the problem in comment #20 have anything to do with the particular bug that
is WONTFIXed here?  Cc'ing tingley.

/be

Comment 22

[C. Daniel Mojoli B.]

I am not sure it is directly related to the problem originally describe since
the tree implementation I am using is the outliner based one, not the old tree.

However, it is at least conceptually related since the summary applies to my
scenario, and I did not want to open a new bug if another preexisted.