34546 - XUL-specified <treeitem> cannot have <treechildren>

Status: RESOLVED FIXED

(Core :: XUL, defect, P3)

Description

[Chris Waterson]

I think that I opened a bug on this a long time ago, but it was either never 
fixed or has regressed.

This XUL does not work:

  <tree>
    <treecol />
    <treechildren>
      <treeitem id="container">
        <treerow>
          <treecell indent="true" value="I have kids!"/>
        </treerow>
        <treechildren>
          <treeitem>
            <treerow>
              <treecell indent="true" value="Test #1.1" />
          </treerow></treeitem>
          <treeitem>
            <treerow>
              <treecell indent="true" value="Test #1.2" />
      </treerow></treeitem></treechildren></treeitem>
      <treeitem>
        <treerow>
          <treecell indent="true" value="Test #2" />
      </treerow></treeitem>
      <treeitem>
        <treerow>
          <treecell indent="true" value="Test #3" />
  </treerow></treeitem></treechildren></tree>

Specifically, the "I have children!" row cannot be opened via double-click.

Even more interesting, specifying "open='true'" on the <treeitem> with the 
id="container" causes the item to be displayed as open (the children 1.1 and 
1.2 are displayed), and the container can be opened and closed with double 
click.

This should work, right? I'm attaching a test case: the left tree has an 
"open='true'" attribute set on the treeitem, the right case does not. Since 
there is no RDF involved here, I presume there must be a bug with the tree 
frame construction code.

Comment 1

[Chris Waterson]

Attachment: test case

Comment 2

[Chris Waterson]

I'm pretty sure that if *this* doesn't work, getting a <template> onto a 
<treeitem> will never work.

Comment 3

[Chris Waterson]

Ok, so I followed the code through to nsTreeRowGroupFrame::AttributeChanged(), 
and it looks like the tree widget just ignores changes to the "open" attribute. 
This seems wrong.

Comment 4

[Chris Waterson]

Ignore previous comment. I don't know what I'm talking about. I did A-B 
comparison between the two trees and determined that in the "initially closed" 
treeitem, nsCSSFrameConstructor::ProcessRestyledFrames() isn't getting 
NS_STYLE_HINT_FRAMECHANGE.

Comment 5

[Chris Waterson]

...because FrameManager::ReResolveStyleContext() has no elements in the 
mUndisplayedMap.

Comment 6

[Chris Waterson]

FWIW, I can get this to work by returning NS_STYLE_HINT_FRAMECHANGE from 
nsXULElement::GetMappedAttributeImpact() for <treeitem open="...">. There don't 
seem to be any evil side effects from this.

Comment 7

[Chris Waterson]

Attachment: proposed fix: force framechange when open changes on <treeitem>

Comment 8

[David Hyatt]

Yeah, somehow the tree widget doesn't get undisplayed elements into that map.  
Knew about it, but don't know why or how to fix it.

Only bad part about doing a framechange is that you'll rebuild the row's 
contents in addition to the kid's, so open/close will get slightly more 
expensive.  Probably as good a workaround as any though.

Comment 9

[David Hyatt]

The fix is fine.  Check it in.

Comment 10

[Chris Waterson]

actually, it's not. it causes bugs like 34408: "double newsgroup".

Comment 11

[David Hyatt]

Great. Then I guess we'll find out why things aren't going into the undisplayed 
map.... accepting for M17.

Comment 12

[Alec Flett]

if you're talking about bug 34726, that's happening anyway, on the nightly
builds (i.e. without your patch)

Comment 13

[Chris Waterson]

Ok, I think I was wrong. This doesn't cause the doubling-up problems, something 
else does.

So, hyatt: if you've got better things to do, I'll just check in the reframe for 
now. I'll open another bug about the map getting horked.

Comment 14

[Chris Waterson]

fix checked in.