Yeah Werner, you should take holidays seriously :) Thanks!
Bruno
On 23 July 2010 13:28, Werner Punz <[email protected]
<mailto:[email protected]>> 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<[email protected] <mailto:[email protected]>> 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<[email protected]
<mailto:[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] <mailto:[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]>
<mailto:[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]>
<mailto:[email protected]
<mailto:[email protected]>>>
To: MyFaces
Development<[email protected]
<mailto:[email protected]>
<mailto:[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]>
<mailto:[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]>
<mailto:[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]>
<mailto:[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]>
<mailto:[email protected]
<mailto:[email protected]>>> wrote:
On Thu, Jul 22, 2010 at 2:14 PM, Matthias
Wessendorf<[email protected]
<mailto:[email protected]>
<mailto:[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]>
<mailto:[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]>
<mailto:[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
