116855 - Modify outliner builder syntax to match outliner content model.

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Jan Varga [:janv]]

Plus add the flag "dont-build-content" to the templates "flags" attribute that
says whether or not to build a content model for tree.

There is a better explanation in the bug 113477.

I propose something like this:
<outliner flex="1">
  <outlinercols>
    <outlinercol id="nameCol" label="Name" flex="1" primary="true"/>
    <outlinercol id="urlCol" label="URL" flex="1"/>
  </outlinercols>
  <outlinerbody flex="1"
  datasources="rdf:bookmarks" ref="NC:BookmarksRoot" flags="dont-build-content">
    <template>
      <rule>
        <outlinerchildren>
          <outlineritem uri="rdf:*">
            <outlinerrow>
              <outlinercell label="rdf:http://home.netscape.com/NC-rdf#Name"/>
 	      <outlinercell label="rdf:http://home.netscape.com/NC-rdf#URL"/>
            </outlinerrow>
          </outlineritem>
        </outlinerchildren>
      </rule>
    </template>
  </outlinerbody>
</outliner>

The "ref" attribute on an <outlinercell> will be optional.

I'm not sure if we should also patch content builder to build content only for
open items.

Comment 1

[Jan Varga [:janv]]

I've got a fix for this in my tree. Now testing.

Comment 2

[Jan Varga [:janv]]

Attachment: patch for review

This patch includes fix for this bug along with bunch of other fixes for
outliner.

Comment 3

[Chris Waterson]

The changes to nsXULOutlinerBuilder look fine, r=waterson on those. I'll defer
to hyatt on the syntax changes and the changes to the outliner builder: you'll
need to get a separate r= and his sr= for that. Also, you'll want to make sure
you get approval for the mailnews changes, cc'ing sspitzer for that. Finally,
I'm not sure I understand the `dont-build-content' flag: how ought I update my
documentation at

  <http://www.mozilla.org/docs/xul/xulnotes/template-outliner.html>.

Comment 4

[Jan Varga [:janv]]

mailnews changes:
Seth, I converted folder pane and stuff to new template syntax.
I also removed "ref" attribute which is now optional.
I noticed that we waste some time by parsing a lot of white spaces. These white
spaces are visible in "properties" definition. So I decided to pack the
properties into one line. It's a bit uglier now, but should be faster.

"dont-build-content" flag:
this flag was suggested by hyatt in bug 113447. It says that we don't want to
build outliner content model (children, item, row, ...), instead we want to use
outliner builder view.

So, when this land, we'll have 3 ways of doing things
1. build content model and use builtin content view
2. use builder view (rdfliner)
3. use own view

Comment 5

[Jan Varga [:janv]]

and finally, there will be no difference between template syntaxes

Comment 6

[Jan Varga [:janv]]

Attachment: testcase.xul

Comment 7

[David Hyatt]

sr=hyatt

Comment 8

[(not reading, please use [e-mail] instead)]

r=sspitzer on the mailnews parts.

Comment 9

[Blake Ross]

Comment on attachment 62737 [details] [diff] [review]
 patch for review

r=blake

Too bad we have to mirror the old, overly complex tree... (<outlineritem>,
<outlinerchildren>)

Comment 10

[Jan Varga [:janv]]

fixed, thanks for reviews!

Comment 11

[Brian Ryner (not reading)]

I just talked to hyatt about this, I don't think we want to require an
<outlinerchildren> inside of <outlinerbody>.  The way that would most mirror the
tree syntax would be to rename everything to <outlinerchildren> and special-case
the outermost <outlinerchildren> to construct a body frame.

Comment 12

[Jan Varga [:janv]]

I did it this way because of nested template content.
I'd have to patch content builder or something, but don't know how at the moment.

Comment 13

[Jan Varga [:janv]]

ah, ok, now I understand
I'll try to do it this way.

Comment 14

[Jan Varga [:janv]]

Duh, this is more involved.
I have some code in my tree, but it'll take more time to get things working.

Comment 15

[Jan Varga [:janv]]

I've got it working, but I had to do many critical changes in the code.
Hope it gets a good shape.

New syntax:
<outliner flex="1"
  datasources="rdf:bookmarks" ref="NC:BookmarksRoot flags="dont-build-content"
  onselect="onSelect()">
  <outlinercols>
    <outlinercol id="nameCol" label="Name" flex="1" primary="true"/>
    <outlinercol id="urlCol" label="URL" flex="1"/>
  </outlinercols>
  <template>
    <rule>
      <outlinerchildren>
        <outlineritem uri="rdf:*">
          <outlinerrow>
            <outlinercell label="rdf:http://home.netscape.com/NC-rdf#Name"/>
            <outlinercell label="rdf:http://home.netscape.com/NC-rdf#URL"/>
          </outlinerrow>
        </outlineritem>
      </outlinerchildren>
    </rule>
  </template>
</outliner>

- outermost <outlinerchildren> has "-moz-box-flex: 1" by default
  and it actaully replaced <outlinerbody>
- all event handlers moved on <outliner>
- content builder, outliner builder and content view use <outliner> as root
  element
- nsIBoxObject has a new attribute "element"
- all pseude CSS rules outlinerbody:foo replaced with outlinerchildren:foo

Comment 16

[Jan Varga [:janv]]

Attachment: big fat patch

this patch also contains fixes for bugs:
94653, 95730, 112238, 118093, 118113, 118154, 118647, 118660, 119751

Comment 17

[David Hyatt]

Comment on attachment 64742 [details] [diff] [review]
big fat patch

sr=hyatt

Comment 18

[Chris Waterson]

r=waterson for the changes in mozilla/content. I didn't look over the XUL/JS or
outliner frame changes, though.

Comment 19

[(not reading, please use [e-mail] instead)]

r=sspitzer on the mailnews parts.

thanks for taking care of (and testing) the mailnews changes.

Comment 20

[Jan Varga [:janv]]

#if defined(XP_MAC)

has to be also changed to

#if defined(XP_MAC) || defined(XP_MACOSX)

nice catch bryner!

Comment 21

[Jan Varga [:janv]]

Attachment: updated patch to the tip, mac defines and fixed DOM inspector

Comment 22

[Blake Ross]

Comment on attachment 65033 [details] [diff] [review]
updated patch to the tip, mac defines and fixed DOM inspector

r=blake

make sure refreshSort is no longer needed. It was needed when I added it.

Comment 23

[Brian Ryner (not reading)]

r=bryner on the OutlinerBodyFrame and OutlinerContentView changes, although XBL
select widgets will be horked until bug 120189 is fixed.

Comment 24

[Joe Hewitt (gone)]

Comment on attachment 65033 [details] [diff] [review]
updated patch to the tip, mac defines and fixed DOM inspector

sr=hewitt

Comment 25

[Jan Varga [:janv]]

fixed