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
