Closed Bug 39054 Opened 25 years ago Closed 24 years ago

make XUL templates work inside of XBL

Categories

(Core Graveyard :: RDF, defect, P3)

defect

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.
Blocks: 38978
Status: NEW → ASSIGNED
Target Milestone: --- → M16
Priority: P3 → P2
not nsbeta2, move to M17
Target Milestone: M16 → M17
Keywords: helpwanted
Target Milestone: M17 → Future
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 )
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...
Darn, I just forgot. Doing that would break XUL templates on HTML elements. So this will require some more thought.
Has there been any more thought on this? It has 5 votes on it, and I really would find this useful very soon. Thanks.
taking.
Assignee: waterson → hyatt
Status: ASSIGNED → NEW
p3/moz0.8
Priority: P2 → P3
Target Milestone: Future → mozilla0.8
->moz0.9
Target Milestone: mozilla0.8 → mozilla0.9
->future
Target Milestone: mozilla0.9 → Future
Attached patch proposed fix (deleted) — Splinter Review
Attached an (untested) fix. I am no XBL hacker: could somebody gimme a test case ?
Assignee: hyatt → waterson
Target Milestone: Future → mozilla0.9
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.
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...
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 :-)
Attached file working test case, part 1, RDF file (deleted) —
Attached file working test case, part 2, XBL file (deleted) —
Attached file working test case, part 3, CSS file (deleted) —
Attached file working test case, part 4, XUL file (deleted) —
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.
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
The above patch makes case (1) work. So now, both case (1) and (2) work, modulo trees being fubar'd somehow.
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.)
Attached file (1) tree widget XBL file (deleted) —
Attached file (2) tree widget CSS file (deleted) —
Attached file (3) tree widget XUL file (deleted) —
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.
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.
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...
Groovy. Thanks for catching those things. I removed `changed': it was cruft left around from earlier experimentation. Fixed up the comment, too.
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
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Only comment I have is that TemplateMap::Put suppresses out of memory errors for no good reason. /be
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.)
Status: RESOLVED → REOPENED
Depends on: 64325, 68681, 68683
Resolution: FIXED → ---
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?
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.
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...
XBL inheritance of datasources dosent work.
Attached patch better patch, sans regressions (deleted) — Splinter Review
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.
seems this checking also caused: bug 68696 "Sidebar Tabs popup listing not visible" bug 68731 "Webtools menu empty in classic and modern" I saw both on linux, but they are gone again in CVS builds from after the backout.
r or sr=hyatt, depending on which you need.
Attached patch address brendan's comments (deleted) — Splinter Review
sr=brendan@mozilla.org, FWIW -- Ben had dibs, per our agreement yesterday in hyatt's cube, but it all looks good to me. /be
Fixed for real this time. (Let's hope.)
Status: REOPENED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
tever is not RDF QA anymore
QA Contact: tever → nobody
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: