MyFaces Core
  1. MyFaces Core
  2. MYFACES-3412

updated AJAX values sometimes delete other elements

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.10, 2.1.4
    • Fix Version/s: 2.0.11, 2.1.5
    • Component/s: None
    • Labels:
      None

      Description

      The attached examples shows a problem I face after updating to the latest 2.0.10/11 and 2.1.4/5 MyFaces versions.

      I have 2 primefaces date pickers inside a h:panelGroup. Both date pickers create ajax requests and refresh their values. If you click one of them (both update the whole panelGroup), the 2nd date picker disapears.

      This might have something to do with the AJAX js rework?

      I can easily work around this issue, but I'm not sure which other problems might arise as well, thus we better fix this

      1. MYFACES-3412-2.patch
        33 kB
        Leonardo Uribe
      2. MYFACES-3412-1.patch
        17 kB
        Leonardo Uribe
      3. ajaxbug.zip
        14 kB
        Mark Struberg

        Activity

        Hide
        Mark Struberg added a comment -

        This sample project shows the bug.

        Show
        Mark Struberg added a comment - This sample project shows the bug.
        Hide
        Werner Punz added a comment - - edited

        Hi Mark, I fixed on the weekend a lifecycle bug which I introduced in the refactoring, it might be also the cause of your problem, update to the latest head to see if the bug is fixed. If not then give me a few days before I start the investigation because i am currently sick and in bed.

        Show
        Werner Punz added a comment - - edited Hi Mark, I fixed on the weekend a lifecycle bug which I introduced in the refactoring, it might be also the cause of your problem, update to the latest head to see if the bug is fixed. If not then give me a few days before I start the investigation because i am currently sick and in bed.
        Hide
        Mark Struberg added a comment -

        Hi Werner! Did this already. Also happens with 2.1.6-SNAPSHOT from r1207510.
        Just do a

        $> mvn clean install -PjettyConfig jetty:run

        on the sample and click on one of the calendar fields. - txs!

        Show
        Mark Struberg added a comment - Hi Werner! Did this already. Also happens with 2.1.6-SNAPSHOT from r1207510. Just do a $> mvn clean install -PjettyConfig jetty:run on the sample and click on one of the calendar fields. - txs!
        Hide
        Mark Struberg added a comment - - edited

        edit oops, mixed up the 2 outputs:

        I did a comparison with my sample with myfaces-2.1.2 and 2.1.5

        The new content (myfaces-2.1.5)

        <input id="mainForm:begin_input" name="mainForm:begin_input" type="text" 
        value="09.11.2011" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript">//<![CDATA[ 
        jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'de_DE',defaultDate:'09.11.2011',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'});});
        //]]><![CDATA[]]]]><![CDATA[></script><div id="mainForm:stayFromErrorPanel"></div>
        </li>
        

        and now the content with myfaces-2.1.2:

        <input id="mainForm:begin_input" name="mainForm:begin_input" type="text" 
        value="09.11.2011" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript"> 
        jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'de_DE',defaultDate:'09.11.2011',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'});});
        </script><div id="mainForm:stayFromErrorPanel"></div>
        </li>
        

        Obviously the new version has a //<![CDATA[ section around the jQuery script. Not sure where this comes from.

        Show
        Mark Struberg added a comment - - edited edit oops, mixed up the 2 outputs: I did a comparison with my sample with myfaces-2.1.2 and 2.1.5 The new content (myfaces-2.1.5) <input id="mainForm:begin_input" name="mainForm:begin_input" type="text" value="09.11.2011" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript">//<![CDATA[ jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'de_DE',defaultDate:'09.11.2011',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'});}); //]]><![CDATA[]]]]><![CDATA[></script><div id="mainForm:stayFromErrorPanel"></div> </li> and now the content with myfaces-2.1.2: <input id="mainForm:begin_input" name="mainForm:begin_input" type="text" value="09.11.2011" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript"> jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'de_DE',defaultDate:'09.11.2011',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'});}); </script><div id="mainForm:stayFromErrorPanel"></div> </li> Obviously the new version has a //<![CDATA[ section around the jQuery script. Not sure where this comes from.
        Hide
        Mark Struberg added a comment -

        Actually I think there is an error in the encoding/escaping of cdata:

        {noscript}
        <?xml version="1.0" encoding="utf-8"?><partial-response><changes><update id="mainForm:timePanel"><![CDATA[<span id="mainForm:timePanel">
        <li><label class="required" for="mainForm:begin">begin</label><span id="mainForm:begin"><input id="mainForm:begin_input" name="mainForm:begin_input" type="text" value="09.11.2011" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript">//<![CDATA[
        jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'de_DE',defaultDate:'09.11.2011',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'});});
        //]]><Unable to render embedded object: File ([CDATA[]]]]><) not found.[CDATA[></script><div id="mainForm:stayFromErrorPanel"></div>
        </li>
        <li><label class="required" for="mainForm:end">end</label><span id="mainForm:end"><input id="mainForm:end_input" name="mainForm:end_input" type="text" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript">//<![CDATA[
        jQuery(function(){widget_mainForm_end = new PrimeFaces.widget.Calendar('mainForm:end', {popup:true,locale:'de_DE',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:end',onSelectUpdate:'mainForm:timePanel'});});
        //]]><Unable to render embedded object: File ([CDATA[]]]]><) not found.[CDATA[></script><div id="mainForm:stayToErrorPanel"></div>
        </li></span>]]></update><update id="javax.faces.ViewState"><![CDATA[8EgC7hvJoXWgMHaUZxk5rx66APlnNueyP32ajDxbvc/i5akMf2jX5eI9FxD0SWjMmqnGuRZk9perBN+pLbsCJY07bYx+qge/7HdPQQiV0a2ev+cfbhVaZm7RvRWDhP3gw8tuWd3LJ4AnHm/Qe9FGsezbMERIR4byQ5ztSHd/RhHBk5ozkptA4s8hSclBEfgd9aYzTDUhBBut/hR8pBOZqWaM27mimcn7Z3vD3Jsn/g+yqjQkuvuFJb7+70X0FL98iPZnIIko9UiTslGoorxCqYYWvPbHDcu0h9b8bIy1hdZbi9jaKnSp6BhQXAwrcKg/Z8OEtFN8EqgXk65ZCBqhQ5PPaWcZmj0rRfVievAalOO3ye1yfyd0dP1Io6WmYZjeNrJ82HZ917wSYuR0WZ4aRsl2IncyYDXzRScIumkzktilLCoP66YmqWQ4iA0lsVkFUzaocn11knWBAqdufjJAOZU68OAilxZHU94YkmMpivIU1FCb8YvCo2aWv7MlvgXYTtEhW4zR/kOrBN+pLbsCJfNxG95mIpFV8pDIkiUQ5znKP0DYFfEbyXfXg39v+EiQdeHWNunUHT6TF0Cl4G+Xi4YpOvK+Wn9+C0hFgjY0Y1inJDJeuFITmZRs+9owxP7M6ArraA==]]></update><extension primefacesCallbackParam="validationFailed">{"validationFailed":false}</extension></changes></partial-response>{noscript}
        Show
        Mark Struberg added a comment - Actually I think there is an error in the encoding/escaping of cdata: {noscript} <?xml version="1.0" encoding="utf-8"?><partial-response><changes><update id="mainForm:timePanel"><![CDATA[<span id="mainForm:timePanel"> <li><label class="required" for="mainForm:begin">begin</label><span id="mainForm:begin"><input id="mainForm:begin_input" name="mainForm:begin_input" type="text" value="09.11.2011" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript">//<![CDATA[ jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'de_DE',defaultDate:'09.11.2011',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'});}); //]]>< Unable to render embedded object: File ([CDATA[]]]]><) not found. [CDATA[></script><div id="mainForm:stayFromErrorPanel"></div> </li> <li><label class="required" for="mainForm:end">end</label><span id="mainForm:end"><input id="mainForm:end_input" name="mainForm:end_input" type="text" class="ui-inputfield ui-widget ui-state-default ui-corner-all" readonly="readonly" /></span><script type="text/javascript">//<![CDATA[ jQuery(function(){widget_mainForm_end = new PrimeFaces.widget.Calendar('mainForm:end', {popup:true,locale:'de_DE',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:end',onSelectUpdate:'mainForm:timePanel'});}); //]]>< Unable to render embedded object: File ([CDATA[]]]]><) not found. [CDATA[></script><div id="mainForm:stayToErrorPanel"></div> </li></span>]]></update><update id="javax.faces.ViewState"><![CDATA [8EgC7hvJoXWgMHaUZxk5rx66APlnNueyP32ajDxbvc/i5akMf2jX5eI9FxD0SWjMmqnGuRZk9perBN+pLbsCJY07bYx+qge/7HdPQQiV0a2ev+cfbhVaZm7RvRWDhP3gw8tuWd3LJ4AnHm/Qe9FGsezbMERIR4byQ5ztSHd/RhHBk5ozkptA4s8hSclBEfgd9aYzTDUhBBut/hR8pBOZqWaM27mimcn7Z3vD3Jsn/g+yqjQkuvuFJb7+70X0FL98iPZnIIko9UiTslGoorxCqYYWvPbHDcu0h9b8bIy1hdZbi9jaKnSp6BhQXAwrcKg/Z8OEtFN8EqgXk65ZCBqhQ5PPaWcZmj0rRfVievAalOO3ye1yfyd0dP1Io6WmYZjeNrJ82HZ917wSYuR0WZ4aRsl2IncyYDXzRScIumkzktilLCoP66YmqWQ4iA0lsVkFUzaocn11knWBAqdufjJAOZU68OAilxZHU94YkmMpivIU1FCb8YvCo2aWv7MlvgXYTtEhW4zR/kOrBN+pLbsCJfNxG95mIpFV8pDIkiUQ5znKP0DYFfEbyXfXg39v+EiQdeHWNunUHT6TF0Cl4G+Xi4YpOvK+Wn9+C0hFgjY0Y1inJDJeuFITmZRs+9owxP7M6ArraA==] ]></update><extension primefacesCallbackParam="validationFailed">{"validationFailed":false}</extension></changes></partial-response>{noscript}
        Hide
        Leonardo Uribe added a comment -

        I checked the response and it is correct. To escape CDATA endings the way to do it is replace ]]> occurrences with:

        ]]><Unable to render embedded object: File ([CDATA[]]]]><) not found.[CDATA[>

        to the ending marker. Later on the client side (javascript) all CDATA blocks are joined together, restoring the markup as it was generated by the server (or renderers). See MYFACES-3339 for details. Note this change was introduced on 2.1.4. We should check the javascript side.

        Show
        Leonardo Uribe added a comment - I checked the response and it is correct. To escape CDATA endings the way to do it is replace ]]> occurrences with: ]]>< Unable to render embedded object: File ([CDATA[]]]]><) not found. [CDATA[> to the ending marker. Later on the client side (javascript) all CDATA blocks are joined together, restoring the markup as it was generated by the server (or renderers). See MYFACES-3339 for details. Note this change was introduced on 2.1.4. We should check the javascript side.
        Hide
        Mark Struberg added a comment -

        Oki I see, we now 'escape' the whole <update> content in the XML and unwrap it at the client.

        Show
        Mark Struberg added a comment - Oki I see, we now 'escape' the whole <update> content in the XML and unwrap it at the client.
        Hide
        Leonardo Uribe added a comment -

        I have checked the demo and the problem is in primefaces. The reason is the javascript code that update the panel is not the one used by myfaces. It's more, the example provided never calls myfaces or mojarra jsf.js.

        But looking this one more carefully, the first time the scripts are rendered like:

        <script type="text/javascript"><!--
        jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin',

        {popup:true,locale:'es_CO',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'}

        );});
        //--></script>

        but on ajax request myfaces replace the comments with a CDATA. This is done because HtmlResponseWriter receive application/xhtml+xml as content type from primefaces, but on normal ajax request from jsf.js receive text/html. From spec point of view there is no doubt MYFACES-3339 is a bug and should be fixed escaping nested CDATA sections.

        The Accept Header used by primefaces is this:

        Accept application/xml, text/xml, /; q=0.01

        But MyFaces send this:

        Accept: text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8

        Later HtmlRenderKitImpl.createResponseWriter() calls HtmlRendererUtils.selectContentType(), so application/xml is recognized as valid content type.

        An easy workaround to this problem is create a RenderKit wrapper and override createResponseWriter() to pass contentTypeListString "text/html" only when primefaces trigger the update.

        In conclusion, I think this issue should be closed as invalid, because no valid workaround can be done from MyFaces side. Instead, the code that updates the panel should be fixed on primefaces javascripts, sending the right header to prevent CDATA inclusion or dealing with xml CDATA blocks (which in my opinion is the right choice). If no objection, I'll close this issue very soon (tomorrow) and start the release of myfaces 2.1.5.

        Show
        Leonardo Uribe added a comment - I have checked the demo and the problem is in primefaces. The reason is the javascript code that update the panel is not the one used by myfaces. It's more, the example provided never calls myfaces or mojarra jsf.js. But looking this one more carefully, the first time the scripts are rendered like: <script type="text/javascript"><!-- jQuery(function(){widget_mainForm_begin = new PrimeFaces.widget.Calendar('mainForm:begin', {popup:true,locale:'es_CO',dateFormat:'dd.mm.yy',changeMonth:true,changeYear:true,formId:'mainForm',url:'/ajaxbug/helloWorld.jsf',hasSelectListener:true,onSelectProcess:'mainForm:begin',onSelectUpdate:'mainForm:timePanel'} );}); //--></script> but on ajax request myfaces replace the comments with a CDATA. This is done because HtmlResponseWriter receive application/xhtml+xml as content type from primefaces, but on normal ajax request from jsf.js receive text/html. From spec point of view there is no doubt MYFACES-3339 is a bug and should be fixed escaping nested CDATA sections. The Accept Header used by primefaces is this: Accept application/xml, text/xml, / ; q=0.01 But MyFaces send this: Accept: text/html,application/xhtml+xml,application/xml;q=0.9, / ;q=0.8 Later HtmlRenderKitImpl.createResponseWriter() calls HtmlRendererUtils.selectContentType(), so application/xml is recognized as valid content type. An easy workaround to this problem is create a RenderKit wrapper and override createResponseWriter() to pass contentTypeListString "text/html" only when primefaces trigger the update. In conclusion, I think this issue should be closed as invalid, because no valid workaround can be done from MyFaces side. Instead, the code that updates the panel should be fixed on primefaces javascripts, sending the right header to prevent CDATA inclusion or dealing with xml CDATA blocks (which in my opinion is the right choice). If no objection, I'll close this issue very soon (tomorrow) and start the release of myfaces 2.1.5.
        Hide
        Werner Punz added a comment - - edited

        The question is why did it work before. Please let me have a look at the client side nevertheless. As you guys know I am currently sick, so please give me until the end of the week that I can have a look at it.
        I just really wanna make sure there is no additional client side bug before rolling the next release.

        Show
        Werner Punz added a comment - - edited The question is why did it work before. Please let me have a look at the client side nevertheless. As you guys know I am currently sick, so please give me until the end of the week that I can have a look at it. I just really wanna make sure there is no additional client side bug before rolling the next release.
        Hide
        Leonardo Uribe added a comment -

        It worked before because PartialResponseWriter had two lines that removed all CDATA sections:

        //we now first remove pending javascript CDATA blocks
        //the reason is if we leave them the ppr chokes on them
        //the syntax from the auto generated CDATA usually is //\s+<![CDATA[

        • resultString = resultString.replaceAll("//\\s*((\\<\\!\\[CDATA\\[)|(\\]\\]
          >))", "");

        In 2.1.4, in MYFACES-3339 this was replaced with:

        + // = Leonardo Uribe = Do this cause a bug on the client side, because
        + // scripts containing '&' will be considered invalid xml.
        + //resultString = resultString.replaceAll("//\\s*((\\<\\!\\[CDATA\\[)|(\\]\\]
        >))", "");

        If no CDATA, primefaces javascript works but with the fix, now there are 2 different CDATA blocks per component and primefaces only handle one of them, trimming the other one.

        Show
        Leonardo Uribe added a comment - It worked before because PartialResponseWriter had two lines that removed all CDATA sections: //we now first remove pending javascript CDATA blocks //the reason is if we leave them the ppr chokes on them //the syntax from the auto generated CDATA usually is //\s+<![CDATA[ resultString = resultString.replaceAll("//\\s*((\\<\\!\\[CDATA\\[)|(\\]\\] >))", ""); In 2.1.4, in MYFACES-3339 this was replaced with: + // = Leonardo Uribe = Do this cause a bug on the client side, because + // scripts containing '&' will be considered invalid xml. + //resultString = resultString.replaceAll("//\\s*((\\<\\!\\[CDATA\\[)|(\\]\\] >))", ""); If no CDATA, primefaces javascript works but with the fix, now there are 2 different CDATA blocks per component and primefaces only handle one of them, trimming the other one.
        Hide
        Werner Punz added a comment - - edited

        I still think the problem is somewhat with us, do we have the right simply to tamper with the markup passed down by the lib? if a <Unable to render embedded object: File (-- ---> is passed down, we should not make a <) not found.[CDATA[ out of it no matter what.
        Especially if <!-- --> is a perfectly fine xhtml construct and in the end is embedded into an outer CDATA section anyway where everything can be placed in unescaped except for ]]>

        We should not be wiser than the user or library which generates the markup.

        Regarding the partial response encoding. The content always is xml which is returned, no matter what the header says, the spec leaves there no option. The actual markup content is in itself in a cdata block.<!-- --> is absolutely valid in CDATA.

        Could it be that the CDATA stuff was added as quick fix for facelets modes where comments were removed? Or that this is an old Facelets bug we have carried around?

        Show
        Werner Punz added a comment - - edited I still think the problem is somewhat with us, do we have the right simply to tamper with the markup passed down by the lib? if a < Unable to render embedded object: File (-- ---> is passed down, we should not make a <) not found. [CDATA[ out of it no matter what. Especially if <!-- --> is a perfectly fine xhtml construct and in the end is embedded into an outer CDATA section anyway where everything can be placed in unescaped except for ]]> We should not be wiser than the user or library which generates the markup. Regarding the partial response encoding. The content always is xml which is returned, no matter what the header says, the spec leaves there no option. The actual markup content is in itself in a cdata block.<!-- --> is absolutely valid in CDATA. Could it be that the CDATA stuff was added as quick fix for facelets modes where comments were removed? Or that this is an old Facelets bug we have carried around?
        Hide
        Leonardo Uribe added a comment -

        The problem is to render the markup for an ajax request two ResponseWriters comes into play:

        org.apache.myfaces.shared.renderkit.html.HtmlResponseWriterImpl
        org.apache.myfaces.context.PartialResponseWriterImpl

        PartialResponseWriter wraps HtmlResponseWriterImpl. JSF 2.0 spec section 13.4.4.1 says this:

        "... JavaServer Faces provides javax.faces.context.PartialResponseWriter to ensure the Ajax response that is written follows the standard format as specified in Section 1.3 “XML Schema Definition for Partial Responses”. Implementations must take care to properly handle nested CDATA sections when writing the response. PartialResponseWriter decorates an existing ResponseWriter implementation by extending javax.faces.context.ResponseWriterWrapper ..."

        Which seems obvious, now take a look at section 13.4.4

        "... The response should be in a common format so JavaScript clients can interpret the markup in a consistent way - an important requirement for component compatability. The agreed upon format and content type for the partial response is XML. This means there should be a ResponseWriter suitable for writing the response in XML ..."

        With this two descriptions we can conclude that MYFACES-3339 is a bug and needs to be fixed. Historically, there was some issues on both ResponseWriter implementations, and the old code was a workaround. But later after solve issues we found the right "combination".

        Now take a look at section 8.1, in the part that talks about createResponseWriter:

        "... The contentTypeList parameter is an "Accept header style" list of content types for this response, or null if the RenderKit should choose the best fit. [P1-start-contentTypeList]The RenderKit must support a value for the contentTypeList argument that comes straight from the Accept HTTP header, and therefore requires parsing according to the specification of the Accept header.[P1-end] Please see Section 14.1 of RFC 2616 (the HTTP 1.1 RFC) for the specification of the Accept header. ..."

        The same description is on the javadoc and basically it means if no contentTypeList the RenderKit should choose the best fit but later says derive it from Accept HTTP heade. Since the header sent by primefaces is:

        Accept application/xml, text/xml, /; q=0.01

        And HtmlRenderKit has two modes xhtml and html, the best match is xml. In theory the client side is the one responsible to send the proper header, because in that place it is possible to know which contentType when the page was rendered at first.

        Since this behavior is specified by the spec, we can't change that part. Since there is a valid workaround adding a RenderKit wrapper, create a config param doesn't sound good.

        It is true send <!-- --> is valid, but according to the spec text/html should be sent into accept header so HtmlResponseWriterImpl could choose the right match.

        Looking more carefully, maybe the problem is we are using xhtml mode for application/xml, text/xml, but it should be only used if Accept header is application/xhtml+xml. I'll check it with more care to see if we have a problem here.

        Show
        Leonardo Uribe added a comment - The problem is to render the markup for an ajax request two ResponseWriters comes into play: org.apache.myfaces.shared.renderkit.html.HtmlResponseWriterImpl org.apache.myfaces.context.PartialResponseWriterImpl PartialResponseWriter wraps HtmlResponseWriterImpl. JSF 2.0 spec section 13.4.4.1 says this: "... JavaServer Faces provides javax.faces.context.PartialResponseWriter to ensure the Ajax response that is written follows the standard format as specified in Section 1.3 “XML Schema Definition for Partial Responses”. Implementations must take care to properly handle nested CDATA sections when writing the response. PartialResponseWriter decorates an existing ResponseWriter implementation by extending javax.faces.context.ResponseWriterWrapper ..." Which seems obvious, now take a look at section 13.4.4 "... The response should be in a common format so JavaScript clients can interpret the markup in a consistent way - an important requirement for component compatability. The agreed upon format and content type for the partial response is XML. This means there should be a ResponseWriter suitable for writing the response in XML ..." With this two descriptions we can conclude that MYFACES-3339 is a bug and needs to be fixed. Historically, there was some issues on both ResponseWriter implementations, and the old code was a workaround. But later after solve issues we found the right "combination". Now take a look at section 8.1, in the part that talks about createResponseWriter: "... The contentTypeList parameter is an "Accept header style" list of content types for this response, or null if the RenderKit should choose the best fit. [P1-start-contentTypeList] The RenderKit must support a value for the contentTypeList argument that comes straight from the Accept HTTP header, and therefore requires parsing according to the specification of the Accept header. [P1-end] Please see Section 14.1 of RFC 2616 (the HTTP 1.1 RFC) for the specification of the Accept header. ..." The same description is on the javadoc and basically it means if no contentTypeList the RenderKit should choose the best fit but later says derive it from Accept HTTP heade. Since the header sent by primefaces is: Accept application/xml, text/xml, / ; q=0.01 And HtmlRenderKit has two modes xhtml and html, the best match is xml. In theory the client side is the one responsible to send the proper header, because in that place it is possible to know which contentType when the page was rendered at first. Since this behavior is specified by the spec, we can't change that part. Since there is a valid workaround adding a RenderKit wrapper, create a config param doesn't sound good. It is true send <!-- --> is valid, but according to the spec text/html should be sent into accept header so HtmlResponseWriterImpl could choose the right match. Looking more carefully, maybe the problem is we are using xhtml mode for application/xml, text/xml, but it should be only used if Accept header is application/xhtml+xml. I'll check it with more care to see if we have a problem here.
        Hide
        Leonardo Uribe added a comment -

        Definitively there is a hole in the spec. In few words, it is not specified how to extract the contentType from Accept HTTP header. But the request has a header that clearly identifies it as an ajax request. We can use that. I think HtmlResponseWriterImpl should have a param to define if the content to render is html or xhtml and that condition should be calculated on HtmlRenderKit, based on the content type and if the request is an ajax request.

        Additionally, it is necessary to define a web config param to set the default mode (html or xhtml), and fix the algorithm that calculates contentType to set that default if no Accept header is set or there is no derived content type.

        Show
        Leonardo Uribe added a comment - Definitively there is a hole in the spec. In few words, it is not specified how to extract the contentType from Accept HTTP header. But the request has a header that clearly identifies it as an ajax request. We can use that. I think HtmlResponseWriterImpl should have a param to define if the content to render is html or xhtml and that condition should be calculated on HtmlRenderKit, based on the content type and if the request is an ajax request. Additionally, it is necessary to define a web config param to set the default mode (html or xhtml), and fix the algorithm that calculates contentType to set that default if no Accept header is set or there is no derived content type.
        Hide
        Leonardo Uribe added a comment -

        Attached proposed patch for this issue. It considers ajax request and resolve contentType correctly in such cases. Adds a web config param:

        org.apache.myfaces.DEFAULT_RESPONSE_WRITER_CONTENT_TYPE_MODE

        to define the default writer content type mode. I tried with the example provided and it works well. If no objections I'll commit this patch very soon.

        Show
        Leonardo Uribe added a comment - Attached proposed patch for this issue. It considers ajax request and resolve contentType correctly in such cases. Adds a web config param: org.apache.myfaces.DEFAULT_RESPONSE_WRITER_CONTENT_TYPE_MODE to define the default writer content type mode. I tried with the example provided and it works well. If no objections I'll commit this patch very soon.
        Hide
        Leonardo Uribe added a comment -

        Added junit test and fix if no valid match throws IllegalArgumentException.

        Show
        Leonardo Uribe added a comment - Added junit test and fix if no valid match throws IllegalArgumentException.

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Mark Struberg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development