[EMAIL PROTECTED] wrote:

mpo 2004/04/09 09:36:00

Modified: src/blocks/forms/java/org/apache/cocoon/forms/transformation
EffectPipe.java
Log:
Fixing the Element.addAttrubute(s). (in its various life-forms)
The implicit understanding of the 'add' of this method was to me:
- ADD if not present yet and
- OVERWRITE if already present.
the previous implementation delegated to AttributesImpl.setAttributes which is in fact
throwing away all existing attributes before adding the new ones.
In combination with the previous patch this was breaking all samples that did not use the form-action and
form-method parameters on forms-transformer (almost all of them)
Revision Changes Path
1.4 +63 -30 cocoon-2.1/src/blocks/forms/java/org/apache/cocoon/forms/transformation/EffectPipe.java
Index: EffectPipe.java
===================================================================
RCS file: /home/cvs/cocoon-2.1/src/blocks/forms/java/org/apache/cocoon/forms/transformation/EffectPipe.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- EffectPipe.java 8 Apr 2004 09:38:55 -0000 1.3
+++ EffectPipe.java 9 Apr 2004 16:36:00 -0000 1.4
@@ -94,53 +94,86 @@
}
}


Please also notice this important msg:

- public void claimAttributes() {
- if (!mine) {
- attrs = new AttributesImpl(attrs);
- mine = true;
- }
- }
+ //TODO: IMPORTANT!!! + // check this: when commenting out this stuff everything still compiles!
+ // this means that attributes on these objects are never explicitely "claimed"
+ // which means that these Element objects should never get referenced + // outside the scope of the SAX event that creates them.
+ // Is that really the case?
+// public void claimAttributes() {
+// if (!mine) {
+// attrs = new AttributesImpl(attrs);
+// mine = true;
+// }
+// }
}


IMHO it could not be the case: AFAIU the whole purpose of the efectpipe is to build a stack of these elements
that lives across SAX-EVENTS, this would mean that the current impl only works with a SAX parser impl that
allocates a new Attributes instance for each sax event (lucky us?)


In that light I'm doubthing if the added complexity of the late attribute-cloning offers us that much.
(doesn't it need to happen anyway?)


IMHO we should consider making the attrs final and thus cloning the lot at Element-constructot-time.

wdot?
-marc=
--
Marc Portier                            http://outerthought.org/
Outerthought - Open Source, Java & XML Competence Support Center
Read my weblog at                http://blogs.cocoondev.org/mpo/
[EMAIL PROTECTED]                              [EMAIL PROTECTED]

Reply via email to