Closed
Bug 39054
Opened 25 years ago
Closed 24 years ago
make XUL templates work inside of XBL
Categories
(Core Graveyard :: RDF, defect, P3)
Core Graveyard
RDF
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9
People
(Reporter: waterson, Assigned: waterson)
References
Details
(Keywords: helpwanted)
Attachments
(21 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/rdf
|
Details | |
(deleted),
text/xml
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/xul
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/xml
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/xul
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/xml
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/xul
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
application/octet-stream
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
XUL templates currently do not work inside of XBL.
Assignee | ||
Updated•25 years ago
|
Assignee | ||
Updated•25 years ago
|
Priority: P3 → P2
Assignee | ||
Updated•24 years ago
|
Keywords: helpwanted
Target Milestone: M17 → Future
Comment 2•24 years ago
|
||
how much work do you think this would be, chris? there are MANY places we could
use this in mail, and end up having to hardcode stuff into overlays... and I'm
sure that this would be a HUGE boon to XUL developers....
(for example, see
http://lxr.mozilla.org/seamonkey/source/mailnews/base/resources/content/msgFolderPickerOverlay.xul
)
Assignee | ||
Comment 3•24 years ago
|
||
This is probably a day's worth of work. The easiest way to do this would be to
change where the XUL template builder is "hooked up" from document creation time
to nsXULElement::SetAttribute(). I don't know what ramifications that might
have...
Assignee | ||
Comment 4•24 years ago
|
||
Darn, I just forgot. Doing that would break XUL templates on HTML elements. So
this will require some more thought.
Comment 5•24 years ago
|
||
Has there been any more thought on this? It has 5 votes on it, and I really
would find this useful very soon.
Thanks.
Assignee | ||
Comment 10•24 years ago
|
||
Assignee | ||
Comment 11•24 years ago
|
||
Attached an (untested) fix. I am no XBL hacker: could somebody gimme a test case
?
Assignee: hyatt → waterson
Target Milestone: Future → mozilla0.9
Comment 12•24 years ago
|
||
Comment 13•24 years ago
|
||
Note: you might need to have this instead for the binding in the XBL file:
<binding id="bookmarks" display="xul:tree"
extends="chrome://global/content/treeBindings.xml#tree">
Apparently there is something special about the display attribute.
Assignee | ||
Comment 14•24 years ago
|
||
Thanks for the test case, Eric. I'm closer, but still not quite there. It looks
like there are two cases that we'll need to handle.
1) The case where the template's root node is completely enclosed
in the XBL binding.
2) The case where the template's root node is in the real content model,
but the <template> is part of the binding. This is what Eric's test case
looks like.
I think (2) might be a bit harder to deal with. I hook up the template builders
before the initial reflow. That means the <template> tag won't be available,
because no frame will have been created for the root node, and the XBL binding
will not have been attached. (Hyatt, is there some magic I could do here to
force the binding to be attached earlier?)
Furthermore, I think that you could probably subsume (2) by (1), just making a
binding that forwards all the attributes down into the binding where the
``real'' root node lives. Anyway, I'm gonna try to make (1) work first, and then
if we think we need (2), let's talk about it...
Comment 15•24 years ago
|
||
Considering the same XUL I had there, modifying the binding to have, in
addition (and not extending tree anymore):
<content>
<xul:tree inherits="ref,context,datasources,multiple">
<template>
...
should be equivalent to my original. The way I did the testcase was basically a
shortcut. It is an important shortcut though, because it is a lot cleaner, in
my opinion anyway.
Man, I can't wait until this is working :-)
Comment 16•24 years ago
|
||
Assignee | ||
Comment 17•24 years ago
|
||
Assignee | ||
Comment 18•24 years ago
|
||
Assignee | ||
Comment 19•24 years ago
|
||
Assignee | ||
Comment 20•24 years ago
|
||
Assignee | ||
Comment 21•24 years ago
|
||
Eric, your test case didn't *quite* work. (But it was *real* close.) There were
at least two problems:
1. Can't load bookmarks in the content area
2. Need to qualify stuff in <content> with the `xul:' namespace prefix.
I went ahead and mangled my template-primer.xul example, and attached it to the
bug (a linked up, hixie style). With this test case, and the, uh, somewhat
larger patch I'm about to attach, I see the content model being properly built
in viewer, but no display. I'll collar hyatt and we can look at it.
FWIW, the patch fixes case (2), but not case (1). To fix case (1), I need to
think a bit more. One more problem with the patch is that I needed to undo
some of the optimizations in the template builder; most notably, the case where
we yank the content out of the content model before tearing apart a tree. (This
will make mail folder switching slower; e.g.) But, it turned out that to make
this work, I needed to fix bug 48154, which is a minor footprint/performance
win.
Assignee | ||
Comment 22•24 years ago
|
||
Assignee | ||
Comment 23•24 years ago
|
||
Assignee | ||
Comment 24•24 years ago
|
||
Assignee | ||
Comment 25•24 years ago
|
||
Assignee | ||
Comment 26•24 years ago
|
||
Okay, so with the above work-in-progress patch, a simpler test case (using only
boxes) works properly. So my guess is that there is some evil frame construction
trickery going on that screws <tree>'s. hyatt, I'll need your love to help me
debug this.
Status: NEW → ASSIGNED
Assignee | ||
Comment 27•24 years ago
|
||
Assignee | ||
Comment 28•24 years ago
|
||
The above patch makes case (1) work. So now, both case (1) and (2) work, modulo
trees being fubar'd somehow.
Assignee | ||
Comment 29•24 years ago
|
||
Ok, hyatt held my hand and we figured out that trees not working simply had to
do with the bevvy of bizarre style rules required to make a tree fly. Changing
the test case to use the ``tree'' tag instead of the ``mytree'' tag, and then
adding back some content made it work. (FWIW, I'll attach the good test case
here.)
Assignee | ||
Comment 30•24 years ago
|
||
Assignee | ||
Comment 31•24 years ago
|
||
Assignee | ||
Comment 32•24 years ago
|
||
Assignee | ||
Comment 33•24 years ago
|
||
Assignee | ||
Comment 34•24 years ago
|
||
Couple comments:
- XUL template builder now maintains state using a hashtable that maps a
generated content element to the template element. We no longer set the
`template' attribute on content elements, so some bogo-hacks in
nsXULContentSink and elsewhere go away. (And I'll be able to close
bug 38978).
- I had to undo an ``optimization'' in
nsXULTemplateBuilder::RemoveGeneratedContent(), which removed the parent
element from the content model before tinkering with the children. Turns out
that this optimization was no longer relevant, since hyatt fixed the tree
control to ignore updates for offscreen rows. (I've verified that this
causes no slowdown in mailnews folder switching.)
Fishing for an r= on the XUL template builder stuff...anyone?
- Previous calls to nsXULDocument::CheckTemplateBuilder() have been
consolidated: it is now only called from AddSubtreeToDocument().
- XBL now calls AddSubtreeToDocument() and RemoveSubtreeFromDocument()
when adding anonymous content to a XUL document.
- Did a bit of cleanup work on the binding manager (at hyatt's request)
to make a single method called GetXBLChildNodesFor() which retrieves all
generated content.
Comment 35•24 years ago
|
||
Looks great! I can't wait to try it out for Jabberzilla.
Now I can make some widgets with datasources. For example, I am going to make a
<presence> widget that can be embedded into a web page... and it will show if
that person is online (now that I have a Jabberzilla component going). Also
planning on doing a <roster> widget of some sort too. Lot's of possibilities.
At the very least, it will clean up my code a lot.
Chris: Looks like you have some Protozilla action going on there? Interesting...
Thanks.
Comment 36•24 years ago
|
||
ok, I don't understand EVERYTHING that's going on, (though now I know how the
contextstack works) but I am glad to see the hashtable instead of template
attribute!
a few minor nits, since that's about all I can offer...
- the "PRBool changed" in nsXULElement looks like something unrelated
- your comment above the element->RemoveChildAt() is now irrelevant :)
other than that, it LOOKS good and you can use me for an r= if nobody else knows
this code either...
Assignee | ||
Comment 37•24 years ago
|
||
Groovy. Thanks for catching those things. I removed `changed': it was cruft left
around from earlier experimentation. Fixed up the comment, too.
Comment 38•24 years ago
|
||
A big oops for me and my deep merging patch (not moving the AddSubtreeToDocument
in Merge to Resolve). I wonder if this resulted in multiple references to the
same element being in the element map (would have been obvious if we had some
sort of publically accessible and used GetElementsById method).
Looks good to me. a=ben@netscape.com
Assignee | ||
Comment 39•24 years ago
|
||
Assignee | ||
Comment 40•24 years ago
|
||
Assignee | ||
Comment 41•24 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 42•24 years ago
|
||
Only comment I have is that TemplateMap::Put suppresses out of memory errors for
no good reason.
/be
Comment 43•24 years ago
|
||
Assignee | ||
Comment 44•24 years ago
|
||
Well, I f***** this one up: broke history and caused all other kinds of
regressions! Backed out changes this AM. I'm gonna add them as dependencies so I
know to check before trying this again. (Also, Bugscape bug 3946.)
Assignee | ||
Comment 45•24 years ago
|
||
Attaching another patch that fixes history. When looking for the <template> tag
in nsXULTemplateBuilder::CompileRules(), I made it look at explicit children
first, then call nsIBindingManager::GetXBLChildNodesFor() if it still hadn't
found a <template>. Apparently, the history window either has anonymous nodes or
a content list that were screwing the pooch here.
hyatt: is this a bug with GetXBLChildNodesFor()? (I.e., if there are both
anonymous and real kids, should it return the union?) Or was this just a bug
with my usage?
Comment 46•24 years ago
|
||
People use XBL with insertion points to move content down deeper into the
content model. They then put <template>s underneath an item that has an
insertion point in its XBL. The <template> gets moved along with all the other
explicit kids deep down into the anonymous content.
Technically XBL has altered the content model so that the template no longer
applies.
Your fix makes sense, although it gets a little weird. Is the <template>
supposed to still apply to the explicit parent, or is it now supposed to apply
to its insertion point parent instead? With your patch, a template could apply
simultaneously to *both*, which is really odd.
Assignee | ||
Comment 47•24 years ago
|
||
So it looks like in the case of the history <tree>, it's finding some XBL
anonymous nodes in nsXBLBinding::GetAnonymousNodes(). Why isn't it finding this
in other trees? Looking...
Comment 48•24 years ago
|
||
XBL inheritance of datasources dosent work.
Assignee | ||
Comment 49•24 years ago
|
||
Assignee | ||
Comment 50•24 years ago
|
||
Latest patch
- Hacks around a possible bug in the XBL insertion point code by forcing
us to scan ``real'' kids for a <template> tag in
nsXULTemplateBuilder::CompileTemplate().
- Fixes ordering problems that regressed bug 64325 (and avoids O(n**2) trawling
through the document adding the same subtree over and over, too). The
specific problem there was that we had something like
<foo id="bar" />
<script>
var bar = document.getElementById("bar");
</script>
...and this was failing because I had postponed all of the
AddSubtreeToDocument() work until after all of a parent nodes' children
had been inserted into the document. The solution is to factor out pre-
and post-order document addition operations, and make the prototype
walking code use them.
Comment 51•24 years ago
|
||
Comment 52•24 years ago
|
||
r or sr=hyatt, depending on which you need.
Assignee | ||
Comment 53•24 years ago
|
||
Comment 54•24 years ago
|
||
sr=brendan@mozilla.org, FWIW -- Ben had dibs, per our agreement yesterday in
hyatt's cube, but it all looks good to me.
/be
Assignee | ||
Comment 55•24 years ago
|
||
Fixed for real this time. (Let's hope.)
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•