On my machine those tests fail now: Failed tests:
teststandardNestingTest(org.apache.myfaces.context.PartialResponseWriterImplTest) testIllegalNestingResolvementTest(org.apache.myfaces.context.PartialResponseWriterImplTest) testIllegalNestingResolvementTest2(org.apache.myfaces.context.PartialResponseWriterImplTest) testStandardUpdate(org.apache.myfaces.context.PartialResponseWriterImplTest) testStandardUpdateNestedCDATA(org.apache.myfaces.context.PartialResponseWriterImplTest) testComponentAuthorNestingFailureTest(org.apache.myfaces.context.PartialResponseWriterImplTest) testStandardInsertAfter(org.apache.myfaces.context.PartialResponseWriterImplTest) testStandardInsertBefore(org.apache.myfaces.context.PartialResponseWriterImplTest) Thus I had to reopen the issue again... Regards, Jakob 2010/7/23 Bruno Aranda <[email protected]> > Hi, > > But the patch had not been checked it yet? Or did you patch it before > trying the tests? > > In any case, I have committed it just now, all myfaces tests pass. > > Cheers! > > Bruno > > > On 23 July 2010 08:26, Matthias Wessendorf <[email protected]> wrote: > >> I just tested 2.0.2-SNAPSHOT with our internal version of ADF Faces, >> that runs on JSF2. >> >> My jetty powered environment gives me no errors with our latest trunk... >> >> -Matthias >> >> On Thu, Jul 22, 2010 at 9:36 PM, Werner Punz <[email protected]> >> wrote: >> > Btw. one issue about this, check if your fix does not break anything in >> the >> > ppr case. >> > The problem is that the ppr responseWriter simply delegates the >> > HtmlResponseWriterImpl, but the issue is >> > that CDATA blocks are automatically opened before any html is rendered, >> any >> > valid CDATA block inside should not be swallowed but escaped in that >> case. >> > >> > >> > So a ppr response looks like this >> > >> > <changes> >> > <update id="bla"><![CDATA[<div id="bla"> .... </div> ]]></update> >> > >> > The problem is that per spec definition the xhr response must be xml >> > and any html must be embedded within CDATA blocks, now if there is >> another >> > CDATA block within the valid html it must be preserved at all costs >> because >> > it is vital that it is properly rendered at the ppr update time (hence >> the >> > complicated escaping in my code) >> > >> > Werner >> > >> > >> > Am 22.07.10 21:31, schrieb Werner Punz: >> >> >> >> Ok point taken, yes the HTMLResponseWriterImpl definitely is html so a >> >> fix there is valid. >> >> >> >> >> >> 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 >> > > -- Jakob Korherr blog: http://www.jakobk.com twitter: http://twitter.com/jakobkorherr work: http://www.irian.at
