We need atomtables for all xslt element- and attribute-names. I suggest similar to how layout does. http://lxr.mozilla.org/mozilla/source/content/shared/src/nsLayoutAtoms.cpp http://lxr.mozilla.org/mozilla/source/content/shared/public/nsLayoutAtoms.h However we maybe don't need to keep track of multiple addrefs/releases
oh, and we need xpath/xslt function names too. Anything more?
I've made a first draft at an implementation. What I don't like about this is that we get extra complexity (and one extra file) just to keep separate tables for XML, XSLT and XPath atoms. IMHO we should just merge them all into a single table. This isn't fully tested yet (i currently can't compile module due to conflicts outside transformiix), but shows the basic flow so that we have something discuss.
Status: NEW → ASSIGNED
Passing this over to peterv since he had conflicting changes in his output rewrite tree
Assignee: kvisco → peterv
Status: ASSIGNED → NEW
the xml table needs "base" as well
...and the xpath table need something for "*"
Attachment #54627 - Attachment is obsolete: true
Attachment #54667 - Attachment is obsolete: true
Attachment #54715 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.6
hrm. this is a bit too much macro stuff for me too. Please get rid of +#define TX_EXPAND_CLASS_NAME(_class) tx##_class##Atoms +#define TX_CALL_EXPAND_CLASS_NAME(_class) TX_EXPAND_CLASS_NAME(_class) +#define TX_ATOM_CLASS_FULLNAME TX_CALL_EXPAND_CLASS_NAME(TX_ATOM_CLASS) as you only need it in one place. In ::init() and ::shutdown() you only need to |_name = ...| instead of |TX_ATOM_CLASS_FULLNAME::_name = ...|. That way you can get rid of the TX_ATOM_CLASS defines too. I like the XML_ATOMS macro though! Maybe addref/release is better then init/shutdown now that it is able to handle multiple calls to either one? How about + _name = TX_GET_ATOM(String(_value)); \ instead of + String _name##String; \ + _name##String.append(_value); \ + TX_ATOM_CLASS_FULLNAME::_name = TX_GET_ATOM(_name##String); \ for standalone?
and change NS_PRECONDITION to NS_ASSERTION since we don't have the former on standalone
-I$(PUBLIC)\raptor should be dead, IIRC. each statement should have a trailing ';', so the macro definitions don't need them. Don't #define TX_IMPL_ATOM_STATICS \ - NamedMap* txAtomService::mAtoms = 0 + NamedMap* txAtomService::mAtoms = 0; as that makes you do TX_IMPL_ATOM_STATICS which should be TX_IMPL_ATOM_STATICS; (same for TX_IMPL_DOM_STATICS, standalone) txAtoms.cpp has a definition of TX_ATOM, that ends with a ';' for standalone, but not for module. You implement the html statics for standalone, too :-( XML_ATOMS';' txAtoms.h /** */, tsstsstss ;-) standalone/Attr.cpp prefix->aPrefix, change the Attr declaration in dom.h, too NodeDefinition.cpp XMLNS_ATTR is dead, if I see that right. (XMLBASE_ATTR could be, too). Just if you like. Rest looks good to me, I'm looking forward to actually using them ;-)
still need to s/NS_PRECONDITION/NS_ASSERTION/, or am I missing something? You also need to add |txHTMLAtoms::init()| in txInit(), or shouldn't we have the html atoms for standalone at all? Agree with Axel, it'll rock to start using this :-)
Attachment #55607 - Attachment is obsolete: true
Attachment #55714 - Attachment is obsolete: true
Comment on attachment 55736 [details] [diff] [review] v3 >Index: source/base/txAtoms.cpp >=================================================================== >+static PRUint32 gHTMLRefCnt = 0; #ifdef this one, too ;-) >Index: source/main/makefile.win and source/main/Makefile.in needs txAtoms.$(OBJ..), too ;-) source/xml/dom/mozImpl/Makefile.in needs includes for xpath and xslt firstname.lastname@example.org
Attachment #55736 - Flags: review+
Attachment #55736 - Attachment is obsolete: true
Comment on attachment 55746 [details] [diff] [review] This should be it r=sicking (apart from some of the build stuff which i don't grok) if you do what Axel said
er, scratch that about what Axel said :)
>In>Index: so>Index: sourcee/txAtoms.cppurce/bas/base/txAtoms.cppurce/base/txAtoms.cppdex: source/base/txAtoms.cpp would that be it? Or should we just scratch JonaonJonas?
Comment on attachment 55746 [details] [diff] [review] This should be it sr=jst
Attachment #55746 - Flags: superreview+
Attachment #55746 - Attachment is obsolete: true
Chickechickechecked in. Passing Jonas. He doesn't like the "include xpath and xslt" to get atoms. Fight! Doinggg!
Assignee: peterv → sicking
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.6 → mozilla0.9.7
actually, i'm not religious enough about this any more...
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
we didn't verify for a long time. I really checked, so VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.