Return-Path: Delivered-To: apmail-myfaces-dev-archive@www.apache.org Received: (qmail 95331 invoked from network); 23 Jul 2010 12:42:12 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 23 Jul 2010 12:42:12 -0000 Received: (qmail 29818 invoked by uid 500); 23 Jul 2010 12:42:12 -0000 Delivered-To: apmail-myfaces-dev-archive@myfaces.apache.org Received: (qmail 29544 invoked by uid 500); 23 Jul 2010 12:42:10 -0000 Mailing-List: contact dev-help@myfaces.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "MyFaces Development" Delivered-To: mailing list dev@myfaces.apache.org Received: (qmail 29537 invoked by uid 99); 23 Jul 2010 12:42:09 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 23 Jul 2010 12:42:09 +0000 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests=FREEMAIL_FROM,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of martin.marinschek@gmail.com designates 209.85.161.53 as permitted sender) Received: from [209.85.161.53] (HELO mail-fx0-f53.google.com) (209.85.161.53) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 23 Jul 2010 12:42:05 +0000 Received: by fxm19 with SMTP id 19so5086439fxm.12 for ; Fri, 23 Jul 2010 05:41:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:content-type:content-transfer-encoding; bh=s/Vma+cobSjbYa6fuTcBMwcqulZCl3fOKuU9nF7UtDk=; b=rdPYj5nMSoBwvrB7wFDsO9MVQR6SKiIJ0mhqOaAXODf/gIowg4UtvU2s1ddwNUDsip zXKqCOkfqdyawxcBNMjoVNI8oG2vwCrbE6Q4LD6IGRGesu1bzErkt3pyMfn8nBy/CpV6 8ssMo7a+nXmtrQdYM8JNYGooBCR4jF7c3Vtr4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:content-type :content-transfer-encoding; b=FAbJYdC6kGB3qlH8DxUEdx8Uw+p/CitMkjpFR+puxFYfJ/BHbsXbyuDiI1N00Pk7E5 41V6dWyYPJR/slV4UkC6Sq9CKjH4/p3QeCsENCk/zrsFoTNfSQV/CQW81dVFYZjJCOdU G/GCT6J9HDkSesuOZuDuqaJwM+hJ56Mh3tjYk= MIME-Version: 1.0 Received: by 10.239.188.209 with SMTP id q17mr317834hbh.52.1279888904013; Fri, 23 Jul 2010 05:41:44 -0700 (PDT) Sender: martin.marinschek@gmail.com Received: by 10.239.148.81 with HTTP; Fri, 23 Jul 2010 05:41:43 -0700 (PDT) In-Reply-To: References: <103180.93488.qm@web27803.mail.ukl.yahoo.com> Date: Fri, 23 Jul 2010 14:41:43 +0200 X-Google-Sender-Auth: IjWMF5FkfWM3AzNtfQR10rhCS2s Message-ID: Subject: Re: Fixing ResponseWriter.startCDATA/endCDATA From: Martin Marinschek To: MyFaces Development Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Fri, Jul 23, 2010 at 2:40 PM, Matthias Wessendorf wr= ote: > On Fri, Jul 23, 2010 at 2:30 PM, Bruno Aranda wro= te: >> 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 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 shoul= d >>> 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 >>>> =A0wrote: >>>>> >>>>> 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 =A0wrot= e: >>>>>> >>>>>> 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 >>>>>> wrote: >>>>>>> >>>>>>> Btw. one issue about this, check if your fix does not break anythin= g >>>>>>> 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 tha= t >>>>>>> case. >>>>>>> >>>>>>> >>>>>>> So a ppr response looks like this >>>>>>> >>>>>>> >>>>>>> =A0 =A0.... =A0]]= > >>>>>>> >>>>>>> The problem is that per spec definition the xhr response must be xm= l >>>>>>> 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 (he= nce >>>>>>> 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 output= s >>>>>>>>> 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 script= s. >>>>>>>>> It >>>>>>>>> is slightly different to the original problem (about the >>>>>>>>> HtmlResponse, >>>>>>>>> which is different from the one in Mojarra). The fix simply check= s >>>>>>>>> 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 t= o >>>>>>>>> change the ResponseWriter or the PartialResponseWriterImpl... >>>>>>>>> >>>>>>>>> Bruno >>>>>>>>> >>>>>>>>> On 22 July 2010 16:59, Werner Punz>>>>>>>> > =A0wrote: >>>>>>>>> >>>>>>>>> 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 ]]> =A0as multiple cdata blocks (the = jira >>>>>>>>> response explains the technique exactly) >>>>>>>>> >>>>>>>>> And yes there is somewhat a speed hit because of this, but in cas= e >>>>>>>>> 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=FCttner doing this by maintaining >>>>>>>>> a reference >>>>>>>>> counter? >>>>>>>>> >>>>>>>>> Another question: how is the performance of all this >>>>>>>>> scanning/dynamic >>>>>>>>> replacement? >>>>>>>>> >>>>>>>>> LieGrue, >>>>>>>>> strub >>>>>>>>> >>>>>>>>> >>>>>>>>> From: Bruno Aranda>>>>>>>> > >>>>>>>>> To: MyFaces Development>>>>>>>> > >>>>>>>>> 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>>>>>>>> > =A0wrote: >>>>>>>>> >>>>>>>>> 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>>>>>>>> > =A0wrote: >>>>>>>>> >>>>>>>>> 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>>>>>>>> > =A0wrote: >>>>>>>>> >>>>>>>>> yeah, sorry, my problem was running only the API >>>>>>>>> tests :) >>>>>>>>> >>>>>>>>> >>>>>>>>> Bruno >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 22 July 2010 13:48, Matthias >>>>>>>>> Wessendorf>>>>>>>> > =A0wrote: >>>>>>>>> >>>>>>>>> On Thu, Jul 22, 2010 at 2:14 PM, Matthias >>>>>>>>> Wessendorf>>>>>>>> > >>>>>>>>> >>>>>>>>> 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>>>>>>>> > >>>>>>>>> >>>>>>>>> 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>>>>>>>> > >>>>>>>>> >>>>>>>>> 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/inde= x.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 > --=20 http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces