myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matthias Wessendorf <mat...@apache.org>
Subject Re: Fixing ResponseWriter.startCDATA/endCDATA
Date Fri, 23 Jul 2010 07:26:00 GMT
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.punz@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.punz@gmail.com
>>> <mailto:werner.punz@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<brunoaranda@gmail.com
>>> <mailto:brunoaranda@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<brunoaranda@gmail.com
>>> <mailto:brunoaranda@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<brunoaranda@gmail.com
>>> <mailto:brunoaranda@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<brunoaranda@gmail.com
>>> <mailto:brunoaranda@gmail.com>> wrote:
>>>
>>> yeah, sorry, my problem was running only the API
>>> tests :)
>>>
>>>
>>> Bruno
>>>
>>>
>>>
>>> On 22 July 2010 13:48, Matthias
>>> Wessendorf<matzew@apache.org
>>> <mailto:matzew@apache.org>> wrote:
>>>
>>> On Thu, Jul 22, 2010 at 2:14 PM, Matthias
>>> Wessendorf<matzew@apache.org
>>> <mailto:matzew@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<matzew@apache.org
>>> <mailto:matzew@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<brunoaranda@gmail.com
>>> <mailto:brunoaranda@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

Mime
View raw message