MyFaces Core
  1. MyFaces Core
  2. MYFACES-3043

Ajax: response(...) function breaks if request(...) has not been called previously

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 2.0.5
    • Component/s: JSR-314
    • Labels:
      None

      Description

      In RichFaces for file upload component we call jsf.ajax.response(...) method to process contents of IFRAME element. The problem is that this call is not preceded by call to jsf.ajax.request(...), making MyFaces fail:

      response : function(request, context)

      { this._q._curReq._response.processResponse(request, context); }

      this._q._curReq - is undefined.

        Activity

        Hide
        Werner Punz added a comment -

        Uups sorry for missing that one I will fix that one as well

        Show
        Werner Punz added a comment - Uups sorry for missing that one I will fix that one as well
        Hide
        Werner Punz added a comment -

        Ok yes this clearly is a bug on our side, because the function since exposed to the outside world should not rely on an active request being present.

        Show
        Werner Punz added a comment - Ok yes this clearly is a bug on our side, because the function since exposed to the outside world should not rely on an active request being present.
        Hide
        Werner Punz added a comment - - edited

        Ok I am working on it, I will take the freedom while fixing this to perform a small refactoring which is needed
        to keep the code maintainable in this part. The change is bigger in the way the response flow is performed.
        All the error, success etc.. functions which currently are assigned at transport level now will be moved into the base request and be overridable via the usual ways. This makes more sense also in a way that the transport.js now is what it should be a factory which generates the correct request type and enqueues it optionally.

        I have to run a lot of tests afterwards so expect the fixup code to be committed sometime next week probably.

        Show
        Werner Punz added a comment - - edited Ok I am working on it, I will take the freedom while fixing this to perform a small refactoring which is needed to keep the code maintainable in this part. The change is bigger in the way the response flow is performed. All the error, success etc.. functions which currently are assigned at transport level now will be moved into the base request and be overridable via the usual ways. This makes more sense also in a way that the transport.js now is what it should be a factory which generates the correct request type and enqueues it optionally. I have to run a lot of tests afterwards so expect the fixup code to be committed sometime next week probably.
        Hide
        Nick Belaevski added a comment -

        Werner,

        Tested with the released MyFaces 2.0.5, works almost fine except the need for '_mfInternal' property to be presented in context passed to response(...) method. I've added it as workaround, but wouldn't it be better to use context.source as JSF specification JSDoc states?

        Show
        Nick Belaevski added a comment - Werner, Tested with the released MyFaces 2.0.5, works almost fine except the need for '_mfInternal' property to be presented in context passed to response(...) method. I've added it as workaround, but wouldn't it be better to use context.source as JSF specification JSDoc states?
        Hide
        Werner Punz added a comment -

        Hi Nick, the _mfInternal is an internal data structure which should not be touched from the outside, context.source should be present as the spec specifies it...

        Show
        Werner Punz added a comment - Hi Nick, the _mfInternal is an internal data structure which should not be touched from the outside, context.source should be present as the spec specifies it...
        Hide
        Werner Punz added a comment -

        just to clarify the situation. I just rechecked the code.
        context.source is present and holds the issuing elem reference
        _mfInternal is as the name says a data holder which is bound to change depending on the implementation version, unfortunately the spec does not say anything about this case and I needed to have this data for a certain bugfix condition and some additional functionality. But just for the short explanation of what can be found there
        var mfInternal = context._mfInternal;

        mfInternal["_mfSourceFormId"] = form.id;
        mfInternal["_mfSourceControlId"] = elementId;
        mfInternal["_mfTransportType"] = transportType;

        mfSourceFormId the issuing form id
        mfSourceControlId the source control id
        mfTransportType the transport type, we allow several types of transport types, this is some preparation work for jsf 2.2 and also enables us to allow multipart iframe based "ajax" submits.

        This is more meta info than context.source can give you, because the spec html clearly states that context.source must have only the source element which then at the request time will be replaced with the dom node for the element. Now our problem was that once you have the node you can run into a situation that the node on the protocol side can be replaced and suddenly you might run into a situation where you cannot reference the nodes parent form anymore, hence i stored this data here.
        mfInternal["_mfSourceControlId"] = elementId; is somewhat redundant but it is wise to drag the parent form id into the context as well (we had a bug raised on detached forms by one of our users)

        But as I said this is internal data and is bound to change in the impl, or will be dropped as soon as the official spec takes care of the form id issue.

        Show
        Werner Punz added a comment - just to clarify the situation. I just rechecked the code. context.source is present and holds the issuing elem reference _mfInternal is as the name says a data holder which is bound to change depending on the implementation version, unfortunately the spec does not say anything about this case and I needed to have this data for a certain bugfix condition and some additional functionality. But just for the short explanation of what can be found there var mfInternal = context._mfInternal; mfInternal ["_mfSourceFormId"] = form.id; mfInternal ["_mfSourceControlId"] = elementId; mfInternal ["_mfTransportType"] = transportType; mfSourceFormId the issuing form id mfSourceControlId the source control id mfTransportType the transport type, we allow several types of transport types, this is some preparation work for jsf 2.2 and also enables us to allow multipart iframe based "ajax" submits. This is more meta info than context.source can give you, because the spec html clearly states that context.source must have only the source element which then at the request time will be replaced with the dom node for the element. Now our problem was that once you have the node you can run into a situation that the node on the protocol side can be replaced and suddenly you might run into a situation where you cannot reference the nodes parent form anymore, hence i stored this data here. mfInternal ["_mfSourceControlId"] = elementId; is somewhat redundant but it is wise to drag the parent form id into the context as well (we had a bug raised on detached forms by one of our users) But as I said this is internal data and is bound to change in the impl, or will be dropped as soon as the official spec takes care of the form id issue.
        Hide
        Werner Punz added a comment -

        Ok now I can see a usecase where the internal params could be problematic. If you want to call the response on your own with your own infrastructure. I will try to make sure the response call can handle the data structures as given in the spec only, so _mfInternal will be ignored if not present.
        If this does not work yet, I will make sure it will work with the next release.

        Show
        Werner Punz added a comment - Ok now I can see a usecase where the internal params could be problematic. If you want to call the response on your own with your own infrastructure. I will try to make sure the response call can handle the data structures as given in the spec only, so _mfInternal will be ignored if not present. If this does not work yet, I will make sure it will work with the next release.
        Hide
        Nick Belaevski added a comment -

        No, this does not work in 2.0.5 - when response(...) is called with our own infrastructure, JS error is thrown. It would be good if we can remove references to those internal structures.

        Show
        Nick Belaevski added a comment - No, this does not work in 2.0.5 - when response(...) is called with our own infrastructure, JS error is thrown. It would be good if we can remove references to those internal structures.
        Hide
        Werner Punz added a comment -

        Yes you are right, I reviewed my own code and it indeed is faulty because it does not take a missing _mfInternal into consideration. I will fix this asap which means you will have the fix in for 2.0.6.
        By definition a direct call to the jsf.ajax.response has to handle a response call with the standard parameters with no internal ones.

        Show
        Werner Punz added a comment - Yes you are right, I reviewed my own code and it indeed is faulty because it does not take a missing _mfInternal into consideration. I will fix this asap which means you will have the fix in for 2.0.6. By definition a direct call to the jsf.ajax.response has to handle a response call with the standard parameters with no internal ones.
        Hide
        Werner Punz added a comment -

        The issue has been shifted over to https://issues.apache.org/jira/browse/MYFACES-3114 and resolved, the jsf.ajax.response now works properly with default input parameters and hence resolves itself with a direct call to response an a custom request.

        Show
        Werner Punz added a comment - The issue has been shifted over to https://issues.apache.org/jira/browse/MYFACES-3114 and resolved, the jsf.ajax.response now works properly with default input parameters and hence resolves itself with a direct call to response an a custom request.
        Hide
        Werner Punz added a comment -

        I reopened the error egain with https://issues.apache.org/jira/browse/MYFACES-3401, I will add a regression test to my testsuite, so that it will never happen again.

        Show
        Werner Punz added a comment - I reopened the error egain with https://issues.apache.org/jira/browse/MYFACES-3401 , I will add a regression test to my testsuite, so that it will never happen again.

          People

          • Assignee:
            Werner Punz
            Reporter:
            Nick Belaevski
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development