XUL templates need to support >1 "substitution property"; for example, using the simple template syntax: <treeitem class="rdf:http://home.netscape.com/NC-rdf#isMessage, rdf:http://home.netscape.com/NC-rdf#isRead" /> Or in the extended syntax: <treeitem class="$isMessage $isRead" /> Besides being nifty, this enables two key optimizations for large templates (like mail/news): 1. It requires fewer attributes to be created on XUL elements, this accounts for the bulk of the expense in time when loading a mailnews folder. 2. It allows the style rules to be written based on class, rather than on attribute. The style system hashes rules by class, so this makes style resolution O(1) rather than O(n). We need to be a bit careful about how we do this to avoid excessive string munging and memory allocation. Ideally, supporting this will not degrade performance of the simple, single-substitution case.

I was looking at this last night, and there's one thing that threw up a red flag for me.. in mail, we have a few extra "static" classes for all treecells, currently in the class attribute as class="treecell-indent tree-cell-threadpane-icon" and so forth. This means that we'll need to have support for both static text and rdf-substituted text within the same attribute, so we should be sure to handle the case where a token in the attribute does not contain rdf:

cc'ing scc for string fu.

Notes to self after re-reading patch... - the case where we run off the end of the string is handled wrong. - need to clarify & think about what might separate a variable name from other content in the string

(I like nsPromiseString) r=rjc (on 2nd cut)

fix checked in

Hey Chris. I have a simple test case (stolen from pref-themes.xul) that shows me that this is working. However, the substituted value for a multi-sustitution is the concatenated string from the matches. In other words: <treecell value="rdf:http://ww...rg/rdf/chrome#name rdf:http://ww..rg/rdf/chrome#display Name"/> gets the result "Classicclassic/1.0" instead of "Classic classic/1.0". That, of course, doesn't give you the right thing, should you want to use these matches to set css class="class-one class-two class-three". I'll attach my testcase (and hopefully it doesn't have some bone-headed mistake).

Believe it or not, this was actually what I intended to do. The reasoning goes something like this: the "end" of the variable is indicated using a space; that space is consumed during processing; if you want to have a space in the resulting template, you need to put *two* spaces in. In retrospect, I think it was a dumb idea! I'll attach a patch shortly that will fix this; if we ever do need the ability to jam something "right up next to" a variable, then we'll add another special character that means "the variable ends here".

What the hell. The above patch gets it right (I think), and fixes another match error to boot. If you really want to concatenate, with no spaces in between, you can terminate your variable name with a caret ("^") character (which, per RFC2396, must be %-encoded if it appears in a URI for real). Otherwise, a variable simply ends when a space is encountered. The math error was a misunderstanding of how nsPromiseSubstring<*> worked: I'd assumed the ctor took a start and end position; it actually takes a start position and a length.

Sorry about that; I was following the old syntax from |Substring|, I think. Is this not intuitive? My bad. In the future, the API will probably change to take a pair of iterators, as with |nsSlidingSubstring| now. A special variation for flat strings will take a pair of character pointers. This combination will be more efficient in every case, and will not encourage confusion since, when using iterators, points and distances have different data types; this is not so when using indexes.

Blessing with a r=rjc.

fix checked in.

no, really!

verified fixed. A space between two RDF URL remains a space, '^' can be used as a catenation operator, and a non-rdf string is not altered as an attribute value, so class="someClass rdf:http...#prop1 rdf:http:...#prop2^rdf:http:...#prop3" appears as class="someClass value1 value2value3" This also works using the advanced syntax for templates. Sweet. (The "^" operator is not consumed when it trails a static value: |foobar^rdf:http://...#prop1| appears as |foobar^value1| although, it is consumed in this case |rdf:http://...#prop1^foobar| appears as |value1foobar| )

In the last case, you can put two carets in; e.g., |rdf:http://...#prop1^^foobar| to produce: |value1^foobar| Thanks for the extensive testing! I'll need to go update the template docs at some point...

Actually, it would be nice to have the ^ operator work in the case of staticname^rdf:..#prop1 producing staticnameValue1 So that a datasource which, for example, has a property with two values, can be convinced to produce a reasonably legible classname: message-status-^rdf:..#prop1 producing message-status-unread

good lord, that would kick ass. I was just starting to think about that. this will allow us to speed up mail just by fixing XUL, not our datasources. Once again, nice job chris!

jgrm: can't you just do "message-status-rdf:..#prop1" to get "message-status-unread". The delimiter is really only needed on the back-end of the variable name so I can know when to stop parsing the RDF property...

Indeed you can (I thought the ' rdf:' was the delimiter on the front end. So you produce classnames like: twostate-rdf:...#p1^-rdf...#p2 -> twostate-true-false

I screwed this up, which I discovered when I was working on bug 53627 :-(. The case that I missed was AddSimpleRuleBindings(), which computes the bindings for the "simple" syntax. Attaching a fix that works, and also cleans up a bunch of copy-n-paste of complicated code.

Removing [nsbeta3+], but we'll need to reconsider if we decide to include the fix for bug 53627...

Had to re-work the patch because egcs-1.1.2 can't stomach operator->*() and the string template foo at the same time. So I made the callbacks be static members of nsXULTemplateBuilder, and explicitly pass a "this" parameter to them. Gonna verify the fancy C++ on gcc-2.7.2.3. Oh, also fixed an ommission where I should've been looking for "..." as well as "rdf:*".

Fix checked in on the trunk. Nominating for RTM because of 53627 dependency.

Marking rtm++. Please check this in as soon as possible.

For posterity's sake (and in case we need to back it out), the last patch is the patch that I am going to check in on the branch: it includes some gcc-2.7.2.3 cleanup, as well as mkaply's OS/2 cleanup.

Fixed on branch.

Verified fixed as far as my pea-brain tells me (at least until waterson finds the next thing to fix :-]). marking vtrunk.