You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Kamal <bh...@tt.com.au> on 2008/06/14 14:16:28 UTC

jx:element patch

Hi,
I am responsible for JIRA issue 2211, including the associated patch. I 
will be honest, I didn't really understand what I did. I knew enough to 
test it and make sure it worked, but there are some points I would like 
to clarify:

Firstly, I was looking at the code for jx:attribute, in particular this:

final Attributes EMPTY_ATTRS = new AttributesImpl();
String elementName = "attribute";

TextSerializer serializer = new TextSerializer();
StringWriter writer = new StringWriter();
serializer.setOutputCharStream(writer);

ContentHandlerWrapper contentHandler = new 
ContentHandlerWrapper(serializer, serializer);
contentHandler.startDocument();

contentHandler.startElement(JXTemplateGenerator.NS, elementName, 
elementName, EMPTY_ATTRS);
Invoker.execute(contentHandler, objectModel, executionContext, 
macroContext, namespaces, this.getNext(), this.getEndInstruction());
contentHandler.endElement(JXTemplateGenerator.NS, elementName, elementName);
contentHandler.endDocument();
valueStr = writer.toString(); 

Am I right in saying that the text serializer is what ensures that XML 
ouput is not serialized in the attributes? I looked at the javadoc for 
TextSerializer and found little useful information.

I noticed that there is very little validation for jx:attribute. You can 
put in any old value for an attribute name, including invalid values 
such as values with spaces and colons (':') in them. I took a very 
different approach for jx:element and tested that the prefix and name 
are valid. Is there are reason why jx:attribute does not check that the 
name is a correct name? Also, in xsp:element, apparently[1], you could 
not specify a namespace without a prefix and visa versa. I chose to 
relax this to just not allowing a prefix without a namespace. Is this right?

This is probably a very stupid question, but I know that 
|SitemapComponentTestCase.generate()produced org.w3c.dom.Document object 
and I was wondering how do I convert this into a string? If I needed to 
debug a test, I would need to install the block then produce output that 
way.
|

Also, I am happy to update the documentation. Do I update the Daisy site 
or is there some other procedure I need to follow? Should I also update 
the samples?

There are a lot more questions I would like answers to, but I think that 
I would be better off investigating on my own.

Finally, I would like to thank the dev team. In the past I don't think I 
would have considered contributing code to Cocoon, not even something 
this simple. Not because I didn't want to but because the development 
environment for Cocoon was a mess. I may not agree with all  the 
decisions made regarding cocoon 2.2, but even I (someone who loathes 
using Maven) have to accept the move to Maven was a necessary change and 
has made the prospect of developing for Cocoon a little less daunting.


Cheers.

[1] http://wiki.apache.org/cocoon/XSPSyntax

Re: jx:element patch

Posted by Leszek Gawron <lg...@mobilebox.pl>.
Grzegorz Kossakowski wrote:
> Kamal pisze:
>> Hi,
>> I am responsible for JIRA issue 2211, including the associated patch. 
>> I will be honest, I didn't really understand what I did. 
> 
> If your patch is a result of random typing then I'm truly impressed. ;-)
> 
>> I knew enough to test it and make sure it worked, but there are some 
>> points I would like to clarify:
>>
>> Firstly, I was looking at the code for jx:attribute, in particular this:
>>
>> final Attributes EMPTY_ATTRS = new AttributesImpl();
>> String elementName = "attribute";
>>
>> TextSerializer serializer = new TextSerializer();
>> StringWriter writer = new StringWriter();
>> serializer.setOutputCharStream(writer);
>>
>> ContentHandlerWrapper contentHandler = new 
>> ContentHandlerWrapper(serializer, serializer);
>> contentHandler.startDocument();
>>
>> contentHandler.startElement(JXTemplateGenerator.NS, elementName, 
>> elementName, EMPTY_ATTRS);
>> Invoker.execute(contentHandler, objectModel, executionContext, 
>> macroContext, namespaces, this.getNext(), this.getEndInstruction());
>> contentHandler.endElement(JXTemplateGenerator.NS, elementName, 
>> elementName);
>> contentHandler.endDocument();
>> valueStr = writer.toString();
>> Am I right in saying that the text serializer is what ensures that XML 
>> ouput is not serialized in the attributes? I looked at the javadoc for 
>> TextSerializer and found little useful information.
> 
> Yep, I guess that TextSerializer implements text output method described 
> for XSLT, see[2]. It means
> that <jx:attribute> will evaluate it's content (descendant elements) and 
> will pull only text result
> of this evaluation. This enables one to use for example macros to 
> generate the value of attribute.
> 
>> I noticed that there is very little validation for jx:attribute. You 
>> can put in any old value for an attribute name, including invalid 
>> values such as values with spaces and colons (':') in them. I took a 
>> very different approach for jx:element and tested that the prefix and 
>> name are valid. 
> 
> Obviously, your approach is much, much better. I appreciate your 
> attention to details.
> 
>> Is there are reason why jx:attribute does not check that the name is a 
>> correct name? 
> 
> I think the only reason is that original authors forgot about 
> implementing these checks. Are you a
> volunteer to fix that? :)
> 
>> Also, in xsp:element, apparently[1], you could not specify a namespace 
>> without a prefix and visa versa. I chose to relax this to just not 
>> allowing a prefix without a namespace. Is this right?
> 
> To be honest, I don't remember why such a rule has been established. 
> Could anyone comment?

It should be totally ok to declare a namespace without a prefix[1]:

<?xml version="1.0"?>
<!-- initially, the default namespace is "books" -->
<book xmlns='urn:loc.gov:books'
       xmlns:isbn='urn:ISBN:0-395-36341-6'>
     <title>Cheaper by the Dozen</title>
     <isbn:number>1568491379</isbn:number>
     <notes>
       <!-- make HTML the default namespace for some commentary -->
       <p xmlns='http://www.w3.org/1999/xhtml'>
           This is a <i>funny</i> book!
       </p>
     </notes>
</book>

[1] http://www.w3.org/TR/REC-xml-names/#defaulting

-- 
Leszek Gawron                         http://www.mobilebox.pl/krs.html
CTO at MobileBox Ltd.


Re: jx:element patch

Posted by Kamal <bh...@tt.com.au>.
Grzegorz Kossakowski wrote:
> Kamal pisze:
>> Hi,
>> I am responsible for JIRA issue 2211, including the associated patch. 
>> I will be honest, I didn't really understand what I did. 
>
> If your patch is a result of random typing then I'm truly impressed. ;-)

Wasn't entirely random, but it was close :)

>> Is there are reason why jx:attribute does not check that the name is 
>> a correct name? 
>
> I think the only reason is that original authors forgot about 
> implementing these checks. Are you a
> volunteer to fix that? :)

I don't mind. If I have the right rules for them I am more than happy to 
update.

>
>> Also, in xsp:element, apparently[1], you could not specify a 
>> namespace without a prefix and visa versa. I chose to relax this to 
>> just not allowing a prefix without a namespace. Is this right?
>
> To be honest, I don't remember why such a rule has been established. 
> Could anyone comment?
>
>> This is probably a very stupid question, but I know that 
>> |SitemapComponentTestCase.generate()produced org.w3c.dom.Document 
>> object and I was wondering how do I convert this into a string? If I 
>> needed to debug a test, I would need to install the block then 
>> produce output that way.
>> |
>
> Take a look at o.a.c.xml.XMLUtils class[3].
>

So simple. Thanks.

>
>> Also, I am happy to update the documentation. Do I update the Daisy 
>> site or is there some other procedure I need to follow? Should I also 
>> update the samples?
>
> Updating samples and documentation is more than welcome! I guess the 
> first place to look at should be following page:
> http://cocoon.apache.org/2.2/blocks/template/1.0/1391_1_1.html

I assume that I should actually update the Daisy docs? What about the 
samples?

>
> I've looked carefully at your patch and it looks ok. I'll commit it 
> tomorrow with minor corrections like methods/file renaming.

Sweet. I will fix the jx:attribute code once this is done. Don't see any 
reason why we shouldn't support a namespace as well. Any thoughts?

>
> Thank you again for your effort!
>
> [2] http://www.w3.org/TR/xslt#section-Text-Output-Method
> [3] 
> http://svn.eu.apache.org/viewvc/cocoon/trunk/core/cocoon-pipeline/cocoon-pipeline-impl/src/main/java/org/apache/cocoon/xml/XMLUtils.java?view=markup 
>
>
>


Re: jx:element patch

Posted by Grzegorz Kossakowski <gr...@tuffmail.com>.
Kamal pisze:
> Hi,
> I am responsible for JIRA issue 2211, including the associated patch. I 
> will be honest, I didn't really understand what I did. 

If your patch is a result of random typing then I'm truly impressed. ;-)

> I knew enough to 
> test it and make sure it worked, but there are some points I would like 
> to clarify:
> 
> Firstly, I was looking at the code for jx:attribute, in particular this:
> 
> final Attributes EMPTY_ATTRS = new AttributesImpl();
> String elementName = "attribute";
> 
> TextSerializer serializer = new TextSerializer();
> StringWriter writer = new StringWriter();
> serializer.setOutputCharStream(writer);
> 
> ContentHandlerWrapper contentHandler = new 
> ContentHandlerWrapper(serializer, serializer);
> contentHandler.startDocument();
> 
> contentHandler.startElement(JXTemplateGenerator.NS, elementName, 
> elementName, EMPTY_ATTRS);
> Invoker.execute(contentHandler, objectModel, executionContext, 
> macroContext, namespaces, this.getNext(), this.getEndInstruction());
> contentHandler.endElement(JXTemplateGenerator.NS, elementName, elementName);
> contentHandler.endDocument();
> valueStr = writer.toString(); 
> 
> Am I right in saying that the text serializer is what ensures that XML 
> ouput is not serialized in the attributes? I looked at the javadoc for 
> TextSerializer and found little useful information.

Yep, I guess that TextSerializer implements text output method described for XSLT, see[2]. It means
that <jx:attribute> will evaluate it's content (descendant elements) and will pull only text result
of this evaluation. This enables one to use for example macros to generate the value of attribute.

> I noticed that there is very little validation for jx:attribute. You can 
> put in any old value for an attribute name, including invalid values 
> such as values with spaces and colons (':') in them. I took a very 
> different approach for jx:element and tested that the prefix and name 
> are valid. 

Obviously, your approach is much, much better. I appreciate your attention to details.

> Is there are reason why jx:attribute does not check that the 
> name is a correct name? 

I think the only reason is that original authors forgot about implementing these checks. Are you a
volunteer to fix that? :)

> Also, in xsp:element, apparently[1], you could 
> not specify a namespace without a prefix and visa versa. I chose to 
> relax this to just not allowing a prefix without a namespace. Is this right?

To be honest, I don't remember why such a rule has been established. Could anyone comment?

> This is probably a very stupid question, but I know that 
> |SitemapComponentTestCase.generate()produced org.w3c.dom.Document object 
> and I was wondering how do I convert this into a string? If I needed to 
> debug a test, I would need to install the block then produce output that 
> way.
> |

Take a look at o.a.c.xml.XMLUtils class[3].

BTW. I think this class should be moved to cocoon-xml-utils module.

> Also, I am happy to update the documentation. Do I update the Daisy site 
> or is there some other procedure I need to follow? Should I also update 
> the samples?

Updating samples and documentation is more than welcome! I guess the first place to look at should 
be following page:
http://cocoon.apache.org/2.2/blocks/template/1.0/1391_1_1.html

> There are a lot more questions I would like answers to, but I think that 
> I would be better off investigating on my own.
> 
> Finally, I would like to thank the dev team. In the past I don't think I 
> would have considered contributing code to Cocoon, not even something 
> this simple. Not because I didn't want to but because the development 
> environment for Cocoon was a mess. I may not agree with all  the 
> decisions made regarding cocoon 2.2, but even I (someone who loathes 
> using Maven) have to accept the move to Maven was a necessary change and 
> has made the prospect of developing for Cocoon a little less daunting.

Thanks Kamal for your kind words. I'm glad that you some positives on Maven transition. :-)

I've looked carefully at your patch and it looks ok. I'll commit it tomorrow with minor corrections 
like methods/file renaming.

Thank you again for your effort!

[2] http://www.w3.org/TR/xslt#section-Text-Output-Method
[3] 
http://svn.eu.apache.org/viewvc/cocoon/trunk/core/cocoon-pipeline/cocoon-pipeline-impl/src/main/java/org/apache/cocoon/xml/XMLUtils.java?view=markup


-- 
Grzegorz Kossakowski