On Fri, Jul 23, 2010 at 2:40 PM, Matthias Wessendorf <mat...@apache.org> wrote: > On Fri, Jul 23, 2010 at 2:30 PM, Bruno Aranda <brunoara...@gmail.com> wrote: >> Yeah Werner, you should take holidays seriously :) > > +1
ESPECIALLY this kind of holidays - a honeymoon ;) best regards, Martin >> On 23 July 2010 13:28, Werner Punz <werner.p...@gmail.com> wrote: >>> >>> Hi I have written a load of tests for the PPR responsewriter to have it >>> covered, but I will do some additional testing mid next week (and will >>> commit those tests) if the ppr responsewriter still behaves as it should >>> after the patch. >>> Since we are not in the middle of another release I can guess the >>> additional testing on the ppr responsewriter can wait a little bit. >>> If anything negative comes out I probably can fix it on the ppr writers >>> side. >>> >>> >>> >>> Werner >>> >>> >>> Am 23.07.10 13:05, schrieb Matthias Wessendorf: >>>> >>>> re-tested; works fine with your patch as well >>>> >>>> On Fri, Jul 23, 2010 at 11:37 AM, Bruno Aranda<brunoara...@gmail.com> >>>> wrote: >>>>> >>>>> 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<mat...@apache.org> 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<werner.p...@gmail.com> >>>>>> 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<werner.p...@gmail.com >>>>>>>>> <mailto:werner.p...@gmail.com>> 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<brunoara...@gmail.com >>>>>>>>> <mailto:brunoara...@gmail.com>> >>>>>>>>> To: MyFaces Development<dev@myfaces.apache.org >>>>>>>>> <mailto:dev@myfaces.apache.org>> >>>>>>>>> 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<brunoara...@gmail.com >>>>>>>>> <mailto:brunoara...@gmail.com>> 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<brunoara...@gmail.com >>>>>>>>> <mailto:brunoara...@gmail.com>> 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<brunoara...@gmail.com >>>>>>>>> <mailto:brunoara...@gmail.com>> wrote: >>>>>>>>> >>>>>>>>> yeah, sorry, my problem was running only the API >>>>>>>>> tests :) >>>>>>>>> >>>>>>>>> >>>>>>>>> Bruno >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 22 July 2010 13:48, Matthias >>>>>>>>> Wessendorf<mat...@apache.org >>>>>>>>> <mailto:mat...@apache.org>> wrote: >>>>>>>>> >>>>>>>>> On Thu, Jul 22, 2010 at 2:14 PM, Matthias >>>>>>>>> Wessendorf<mat...@apache.org >>>>>>>>> <mailto:mat...@apache.org>> >>>>>>>>> >>>>>>>>> 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<mat...@apache.org >>>>>>>>> <mailto:mat...@apache.org>> >>>>>>>>> >>>>>>>>> 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<brunoara...@gmail.com >>>>>>>>> <mailto:brunoara...@gmail.com>> >>>>>>>>> >>>>>>>>> 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 >>>>> >>>>> >>>> >>>> >>>> >>> >>> >> >> > > > > -- > Matthias Wessendorf > > blog: http://matthiaswessendorf.wordpress.com/ > sessions: http://www.slideshare.net/mwessendorf > twitter: http://twitter.com/mwessendorf > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces
