No please, it is passing now!! Forgive my mistake of re-using a constant with a line return in it!
It was a possible bug with MyFaces, really, because the methods to add CDATA in the comments where not relying on startCDATAs or checking nested CDATAs, which can be a potential bug in my opinion. Bruno On 23 July 2010 11:46, Matthias Wessendorf <[email protected]> wrote: > I get the build failure as well... > LEt's revert the commit and ask Primefaces guy(s) to fix it.... > > BTW It is not the first issues that users find when running the lib > with MyFaces... > > Honestly, I don't understand why he does not even bother to test > against MyFaces, > especially since he is a committer :-) > > -Matthias > > On Fri, Jul 23, 2010 at 12:28 PM, Werner Punz <[email protected]> > wrote: > > Hi I have to add my 2c to the entire issue. > > The main issue still is mainly at Primefaces and here is the reason, > > primefaces uses an xml response type protocol in the background but does > not > > escape the cdata blocks like we and mojarra do in the ppr part. > > Instead it just uses the standard response writer to emit xml and expects > a > > certain behavior type which is not clearly specified, to the worse I > expect > > that it runs still into some problems because of missing fixup code on > its > > side. > > > > Now I am doing some guessin ghere: > > > > Now if we suppress nested CDATA blocks there still will be usecases where > > primefaces will fail (Just a guess, Primefaces might have fixed it, so if > I > > am wrong then beat me for it). The problem simply is if you use xml as > > response format with a suppressing behavior: > > > > > > imaging following valid html codeblock > > > > <script> > > //<![CDATA[ > > alert("<hello world>"); > > //]]> > > </script> > > > > or following > > <script> > > //<!-- > > alert("]]>"); > > //--> > > </script> > > > > Now what happens is following if we embed the first case and suppress the > > CDATA in our standard response writer > > > > <![CDATA[ > > <script> > > // > > alert("<hello world>"); > > // > > </script> > > ]> > > > > Which is very likely to fail in a parser in strict xhtml because > > after the ppr processing following is sent to the engine: > > <script> > > // > > alert("<hello world>"); > > // > > </script> > > > > The correct output would be here: > > <![CDATA[ > > <script> > > //<![CDATA[ > > alert("<hello world>"); > > //]]><![CDATA[]]]]><![CDATA[[> > > </script> > > ]]> > > > > > > The second case is even worse because it results on primefaces side im > > <![CDATA[ > > <script> > > //<!-- > > alert("]]>"); > > //--> > > </script> > > ]]> > > which is an absolute no go in xml because you can embed everything into > > CDATA blockes except ]]> > > > > unless Primefaces does not do its own escaping on the strings (which it > very > > unlikely does not do. > > > > The correct output on this would look like: > > > > The second case is even worse because it results on primefaces side im > > <![CDATA[ > > <script> > > //<!-- > > alert("]]><![CDATA[]]]]><![CDATA[[>"); > > //--> > > </script> > > ]> > > > > But both correct responses are only correct if you expect xml which in > > itself embeds html code, so this is a no go for a standard response > writer > > which is expected just to emit xhtml/html code with nothing embedded. > > > > So as I said the problem is not as easy at the first look and the > ultimate > > fix must happen on the primefaces side here. > > > > The problem simply is if you use the html response writer for straight > xml > > encoding with embedded html what is happening here, then you run into > those > > problems, hence we have our own ppr response writer and also mojarra > which > > exactly deals with those kinds of problems on ppr level. > > > > Dont get me wrong I dont mind having a fix for nested CDATA blocks in the > > html response writer, since the html response writer is for html this is > > correct, but that won´t resolve all problems primefaces has here, because > it > > has to take into consideration of all this (and I have an eery feeling > that > > the suppressing fix on the Myfaces side has to be disabled for the ppr > case > > so that the ppr fixup code works properly because it delegates the html > > response writer and fixes the issue afterwards) > > > > > > > > > > Werner > > > > > > > > Am 22.07.10 20:11, schrieb Bruno Aranda: > >> > >> But we are talking about the HtmlResponseWriterImpl, which outputs HTML. > >> The fix I have done is in that class and it only checks if there is a > >> CDATA already started before starting one when writing the scripts. It > >> is slightly different to the original problem (about the HtmlResponse, > >> which is different from the one in Mojarra). The fix simply checks if > >> there is the CDATA around the script before opening another one inside > >> the script. I think it is OK if we just do not nest CDATAs in the HTML > >> response, even if it was allowed. > >> > >> And this fixes the problems with PrimeFaces too, without having to > >> change the ResponseWriter or the PartialResponseWriterImpl... > >> > >> Bruno > >> > >> On 22 July 2010 16:59, Werner Punz <[email protected] > >> <mailto:[email protected]>> wrote: > >> > >> Hia guys please also read up on my jira response. > >> The entire thing is not as easy as it seems, because there are > >> various ways a cdata block can be opened, first you can do it via > >> startCDATA secondly you can do it via a direct write. > >> > >> I did some kind of double buffering in case of a cdata block was > >> opened and then escaped the ]]> as multiple cdata blocks (the jira > >> response explains the technique exactly) > >> > >> And yes there is somewhat a speed hit because of this, but in case > >> of the partial response writer I did not have a chance because: > >> > >> But the partial response writer is somewhat different, because it > >> has to press html code in a valid xml response format, so nested > >> cdata blocks can occur naturally, the normal response writer is > >> somewhat different because nested cdata blocks are only forbidden > >> for xmlish output dialects others might allow it. > >> > >> Werner > >> > >> > >> > >> Am 22.07.10 17:47, schrieb Mark Struberg: > >> > >> > >> But isn't the patch of Marcus Büttner doing this by maintaining > >> a reference > >> counter? > >> > >> Another question: how is the performance of all this > >> scanning/dynamic > >> replacement? > >> > >> LieGrue, > >> strub > >> > >> > >> From: Bruno Aranda<[email protected] > >> <mailto:[email protected]>> > >> To: MyFaces Development<[email protected] > >> <mailto:[email protected]>> > >> Sent: Thu, July 22, 2010 5:26:35 PM > >> Subject: Re: Fixing ResponseWriter.startCDATA/endCDATA > >> > >> Further investigation of this incompatibility problem with > >> myfaces leads me to > >> the fact that in the HtmlResponseWriterImpl, when we write > >> the content of a > >> script, we create a CDATA element without checking if is > >> nested at all. That is > >> > >> > >> a problem, because if we use the standard response writer > >> and we write a script > >> > >> > >> section inside a CDATA section, the problem will be > >> triggered... > >> > >> We need a way in HtmlResponseWriterImpl to check nested > >> CDATA calls to the > >> startCDATA or endCDATA methods I guess. > >> > >> Cheers, > >> > >> Bruno > >> > >> > >> On 22 July 2010 15:15, Bruno Aranda<[email protected] > >> <mailto:[email protected]>> wrote: > >> > >> Just clicked on sent and Werner had answered in the JIRA > >> issue explaining the > >> partial approach... > >> > >> > >> Cheers, > >> > >> Bruno > >> > >> > >> > >> On 22 July 2010 15:12, Bruno > >> Aranda<[email protected] > >> <mailto:[email protected]>> wrote: > >> > >> As you can see in my black box tests with Mojarra, the > >> behaviour is different in > >> > >> both implementations. In the base ResponseWriter class, > >> they don't do anything > >> > >> > >> in the startCDATA method and throw an undocumented > >> exception in the endCDATA. > >> > >> > >> In both implementations of the base class, they > >> throw an exception if the > >> startCDATA method is called and it had been called > >> already... > >> > >> I don't quite understand our implementation of the > >> PartialResponseWriterImpl. We > >> > >> do buffer nested CDATAs and write them when closing > >> the parent one? This would > >> > >> > >> still create nested CDATAs... I still need to > >> understand this bit properly, > >> > >> Cheers, > >> > >> Bruno > >> > >> > >> > >> On 22 July 2010 13:58, Bruno > >> Aranda<[email protected] > >> <mailto:[email protected]>> wrote: > >> > >> yeah, sorry, my problem was running only the API > >> tests :) > >> > >> > >> Bruno > >> > >> > >> > >> On 22 July 2010 13:48, Matthias > >> Wessendorf<[email protected] > >> <mailto:[email protected]>> wrote: > >> > >> On Thu, Jul 22, 2010 at 2:14 PM, Matthias > >> Wessendorf<[email protected] > >> <mailto:[email protected]>> > >> > >> wrote: > >> > >> so, maybe there are now regressions? > >> > >> hrm. have you done some testing? > >> > >> > >> Ah, the discussion is on the JIRA.. > >> > >> please run tests, before committing ;-) > >> > >> > >> > >> -M > >> > >> On Thu, Jul 22, 2010 at 2:07 PM, > >> Matthias Wessendorf<[email protected] > >> <mailto:[email protected]>> > >> > >> wrote: > >> > >> sounds right. > >> > >> does blame say more why it does not > >> do nothing? > >> > >> It is also kinda strange since the > >> TCK was successfully executed for > >> 2.0.0 and 2.0.1; > >> > >> -Matthias > >> > >> On Thu, Jul 22, 2010 at 1:48 PM, > >> Bruno Aranda<[email protected] > >> <mailto:[email protected]>> > >> > >> wrote: > >> > >> Hi, > >> > >> Having problems with Primefaces > >> again I have realised that > >> something > >> > >> was > >> > >> working with Mojarra, but not > >> with MyFaces. Again, is the > >> ResponseWriter.startCDATA stuff > >> which Primefaces invokes directly > >> on > >> > >> its > >> > >> main phase listener. > >> > >> However, reading the javadocs: > >> > >> > >> > https://javaserverfaces.dev.java.net/nonav/docs/2.0/javadocs/index.html > >> > >> It says that method "should > >> take no action when invoked"... > >> which > >> > >> means > >> > >> that it should be completely > >> empty as far as I understand. If > >> that was > >> > >> the > >> > >> case, we would get the same > >> behaviour in both > >> implementations... > >> > >> Cheers, > >> > >> Bruno > >> > >> > >> > >> > >> -- > >> Matthias Wessendorf > >> > >> blog: > >> > >> http://matthiaswessendorf.wordpress.com/ > >> sessions: > >> > http://www.slideshare.net/mwessendorf > >> twitter: > http://twitter.com/mwessendorf > >> > >> > >> > >> > >> -- > >> Matthias Wessendorf > >> > >> blog: > >> http://matthiaswessendorf.wordpress.com/ > >> sessions: > >> http://www.slideshare.net/mwessendorf > >> twitter: http://twitter.com/mwessendorf > >> > >> > >> > >> > >> -- > >> > >> Matthias Wessendorf > >> > >> blog: > http://matthiaswessendorf.wordpress.com/ > >> sessions: > http://www.slideshare.net/mwessendorf > >> twitter: http://twitter.com/mwessendorf > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > > > > > > > > > > -- > Matthias Wessendorf > > blog: http://matthiaswessendorf.wordpress.com/ > sessions: http://www.slideshare.net/mwessendorf > twitter: http://twitter.com/mwessendorf >
