MyFaces Core
  1. MyFaces Core
  2. MYFACES-3401

Issue with RichFaces fileUpload component using MyFaces 2.0

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.9
    • Fix Version/s: 2.0.12, 2.1.6
    • Component/s: JSR-314
    • Labels:
      None

      Description

      I have an application that I'll attach to this issue that can reproduce this issue. To reproduce you need to:

      1) Install webapp.war

      2) navigate to index.xhtml

      3) click the "add" button and select a file to upload ( any file it does not matter )

      4) click the "upload" button

      The following error occurs: context.source is undefined -> jsf.js.jsf:6422

      If I look at the jsf.js line 6422 I see the following:

      var elementId = (context._mfInternal)? context._mfInternal["_mfSourceControlId"] : context.source.id;

      The above line is in the processUpdate : function(request, context, node) method. I'm not very familiar with the JS within MyFaces so I wanted to open up an issue and get some feedback from the community.

      I see that other issues have been opened regarding RichFaces fileUpload such as : https://issues.apache.org/jira/browse/MYFACES-3043

      1. Patch_for_Richfaces_4_0_fileupload.patch
        3 kB
        Werner Punz
      2. webapp.war
        3.09 MB
        Paul Nicolucci

        Activity

        Hide
        Paul Nicolucci added a comment -

        Test application for reproduction of the issue.

        Show
        Paul Nicolucci added a comment - Test application for reproduction of the issue.
        Hide
        Werner Punz added a comment -

        I will have a look at it sometime this week or early next week.
        Sorry for picking it up so late, but this bug went under my radar since it was not marked as an ajax or jsf bug.

        Show
        Werner Punz added a comment - I will have a look at it sometime this week or early next week. Sorry for picking it up so late, but this bug went under my radar since it was not marked as an ajax or jsf bug.
        Hide
        Werner Punz added a comment - - edited

        Ok I did a quick preliminary investigation, the issue is that Richfaces basically introduces for the Ajax upload its own request cycle and then calls the response with an empty context. Now I don´t rely on that case because I assume that the context is always filled at least with one source element. This comes implicitely from the fact that if no source is given also the viewstate update would fail in certain cases.

        Now the spec clearly states that following must be given to the response:

        context
        An object containing the request context, including the following properties: the source element, per call onerror callback function, and per call onevent callback function. However onerror and onevent can be empty so it is not clear whether source also can be empty.

        The problem here is the viewstate update, having no information where to apply it (by the missing source and render targets) there is only one way to apply it from our side, only if one form is given. Maybe however Richfaces does the Viewstate update itself.

        Either way I will do more investigation and then fix this either on our or on the side of Richfaces.

        Show
        Werner Punz added a comment - - edited Ok I did a quick preliminary investigation, the issue is that Richfaces basically introduces for the Ajax upload its own request cycle and then calls the response with an empty context. Now I don´t rely on that case because I assume that the context is always filled at least with one source element. This comes implicitely from the fact that if no source is given also the viewstate update would fail in certain cases. Now the spec clearly states that following must be given to the response: context An object containing the request context, including the following properties: the source element, per call onerror callback function, and per call onevent callback function. However onerror and onevent can be empty so it is not clear whether source also can be empty. The problem here is the viewstate update, having no information where to apply it (by the missing source and render targets) there is only one way to apply it from our side, only if one form is given. Maybe however Richfaces does the Viewstate update itself. Either way I will do more investigation and then fix this either on our or on the side of Richfaces.
        Hide
        Werner Punz added a comment - - edited

        Hia, I just ran the same test with richfaces 4.1 and apparently they have fixed the issue from their side. Apparently they also added our internal params, which they would not have needed.
        So the easy fix is to switch to Richfaces 4.1.

        Nevertheless it needs to be cleared up whether the context.source parameter is mandatory or not, I am already in contact with Ed Burns regarding this and will fix our codebase accordingly.

        Show
        Werner Punz added a comment - - edited Hia, I just ran the same test with richfaces 4.1 and apparently they have fixed the issue from their side. Apparently they also added our internal params, which they would not have needed. So the easy fix is to switch to Richfaces 4.1. Nevertheless it needs to be cleared up whether the context.source parameter is mandatory or not, I am already in contact with Ed Burns regarding this and will fix our codebase accordingly.
        Hide
        Werner Punz added a comment - - edited

        Ok we still have an error in the codebase, even if we interprete the stack narrow a context:{source:{id:<componentId>}} should go through because all our _mfInternal values are purely optional. Now we are getting an error in that case.
        This needs to be fixed and we probably also once this is fixed can deal with the context:{} case for richfaces 4.0 easily, no matter what the result is. a context:{} simply probably should response the xml but should skip the viewstate application on the origin elements form, according to the logic of the code. In case of richfaces 4.0 the viewstate application is not needed because it is not altered during the fileupload anyway (richfaces intercepts the request processes the multipart request drops the file into a temp dir and then once done pushes a another ajax request over the normal lifecycle to process the rest of the ajax handling.

        Show
        Werner Punz added a comment - - edited Ok we still have an error in the codebase, even if we interprete the stack narrow a context:{source:{id:<componentId>}} should go through because all our _mfInternal values are purely optional. Now we are getting an error in that case. This needs to be fixed and we probably also once this is fixed can deal with the context:{} case for richfaces 4.0 easily, no matter what the result is. a context:{} simply probably should response the xml but should skip the viewstate application on the origin elements form, according to the logic of the code. In case of richfaces 4.0 the viewstate application is not needed because it is not altered during the fileupload anyway (richfaces intercepts the request processes the multipart request drops the file into a temp dir and then once done pushes a another ajax request over the normal lifecycle to process the rest of the ajax handling.
        Hide
        Werner Punz added a comment -

        The issue now is fixed in the latest head of both 2.0 and 2.1 the richfaces 4.0 file upload component now works sufficiently.

        Show
        Werner Punz added a comment - The issue now is fixed in the latest head of both 2.0 and 2.1 the richfaces 4.0 file upload component now works sufficiently.
        Hide
        Werner Punz added a comment -

        I also will add a regression test to the ajax testsuite on apache extras, so that it will never happen again.

        Show
        Werner Punz added a comment - I also will add a regression test to the ajax testsuite on apache extras, so that it will never happen again.
        Hide
        Werner Punz added a comment -

        found another small issue in my fixes

        Show
        Werner Punz added a comment - found another small issue in my fixes
        Hide
        Werner Punz added a comment -

        This is a patch for the latest myfaces 2.1.6 stable codebase which fixes the issue, apply if needed.

        Show
        Werner Punz added a comment - This is a patch for the latest myfaces 2.1.6 stable codebase which fixes the issue, apply if needed.

          People

          • Assignee:
            Werner Punz
            Reporter:
            Paul Nicolucci
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development