MyFaces Core
  1. MyFaces Core
  2. MYFACES-3002

FaceletComponsitionContextImpl drops viewParams

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.0.3
    • Fix Version/s: 2.0.4
    • Component/s: JSR-314
    • Labels:
      None

      Description

      This is related to MYFACES-2774

      FaceletComponsitionContextImpl#finalizeForDeletion drops the 'javax_faces_metadata' from the UIViewRoot s _facetMap. Thus all 'old' viewParams are not available for propagation to the next view anymore.

      This situation happens if an action returns something like
      > return "nextPage.xhtml??faces-redirect=true&includeViewParams=true";

        Issue Links

          Activity

          Hide
          Jakob Korherr added a comment -

          The code committed on MYFACES-2774 causes the f:metadata facet beeing removed from UIViewRoot on vdl.buildView(), because FaceletViewDeclarationLanguage.isBuildingViewMetadata(ctx.getFacesContext()) is false and thus f:metadata is not processed in DefaultFacelet.apply(). As a result of this, myFaceletContext.finalizeForDeletion(parent); removes the f:metadata facet and all metadata information (including view-parameters) is lost.

          Unfortunately this has a lot of side effects. Apart from the problems Mark is having, very basic examples do not work any more since MYFACES-2774 (which is actually "funny", because no-one discovered it for months...).

          For example if you have a f:viewParam which stores its value in a @RequestScoped managed bean (e.g. #{requestBean.input}} you will loose the value of the view-parameter immediatly when doing a post-back, because its value is 1) never stored in the state and 2) wouldn't even get picked up if it was in the state.

          This means in MyFaces core 2.0.2 and 2.0.3 view-parameters rely on the fact that their value is still available from the model in the next request. If it is not available in the model in the next request (e.g. when using a @RequestScoped managed bean), it will be lost.

          Show
          Jakob Korherr added a comment - The code committed on MYFACES-2774 causes the f:metadata facet beeing removed from UIViewRoot on vdl.buildView(), because FaceletViewDeclarationLanguage.isBuildingViewMetadata(ctx.getFacesContext()) is false and thus f:metadata is not processed in DefaultFacelet.apply(). As a result of this, myFaceletContext.finalizeForDeletion(parent); removes the f:metadata facet and all metadata information (including view-parameters) is lost. Unfortunately this has a lot of side effects. Apart from the problems Mark is having, very basic examples do not work any more since MYFACES-2774 (which is actually "funny", because no-one discovered it for months...). For example if you have a f:viewParam which stores its value in a @RequestScoped managed bean (e.g. #{requestBean.input}} you will loose the value of the view-parameter immediatly when doing a post-back, because its value is 1) never stored in the state and 2) wouldn't even get picked up if it was in the state. This means in MyFaces core 2.0.2 and 2.0.3 view-parameters rely on the fact that their value is still available from the model in the next request. If it is not available in the model in the next request (e.g. when using a @RequestScoped managed bean), it will be lost.
          Hide
          Jakob Korherr added a comment -

          changed to critical

          Show
          Jakob Korherr added a comment - changed to critical
          Hide
          Leonardo Uribe added a comment -

          I don't think this one is related to the changes done on MYFACES-2774. Instead, I think this one is an use case not considered. Could you provide an example for this one, so I can check it and see how we can solve it?

          Show
          Leonardo Uribe added a comment - I don't think this one is related to the changes done on MYFACES-2774 . Instead, I think this one is an use case not considered. Could you provide an example for this one, so I can check it and see how we can solve it?
          Hide
          Mark Struberg added a comment -

          It most probably is, because our internally released version is from 29. august 2010 and 2.0.2 already crashed for me (and had a few other bugs which got fixed in the meantime too).
          I debugged into it and the problem is really that FaceletCompositionContextImpl#finalizeForDeletion() drops the javax_faces_metadata from the ViewRoots _facetMap. After that the viewParam information from the restoreView phase is gone...

          Show
          Mark Struberg added a comment - It most probably is, because our internally released version is from 29. august 2010 and 2.0.2 already crashed for me (and had a few other bugs which got fixed in the meantime too). I debugged into it and the problem is really that FaceletCompositionContextImpl#finalizeForDeletion() drops the javax_faces_metadata from the ViewRoots _facetMap. After that the viewParam information from the restoreView phase is gone...
          Hide
          Jakob Korherr added a comment -

          IMHO (and after a lot of debugging) it really comes from MYFACES-2774.

          However, I commited an example that shows this issue to the test-webapp (viewparam.xhtml). Instructions are included.

          Show
          Jakob Korherr added a comment - IMHO (and after a lot of debugging) it really comes from MYFACES-2774 . However, I commited an example that shows this issue to the test-webapp (viewparam.xhtml). Instructions are included.
          Hide
          Leonardo Uribe added a comment -

          I committed an alternative fix. I'm still not convinced about this issue is related to MYFACES-2774, because there is only circunstancial evidence.

          Show
          Leonardo Uribe added a comment - I committed an alternative fix. I'm still not convinced about this issue is related to MYFACES-2774 , because there is only circunstancial evidence.
          Hide
          Mark Struberg added a comment -

          sorry Leo, your commit breaks the behaviour again.
          You can simply check this with the test application Jakob did provide. It now also crashes again in my real world project. But I agree that there might be better ways to solve that than the quick-fix. So I'll revert your patch and go with that for now.

          Show
          Mark Struberg added a comment - sorry Leo, your commit breaks the behaviour again. You can simply check this with the test application Jakob did provide. It now also crashes again in my real world project. But I agree that there might be better ways to solve that than the quick-fix. So I'll revert your patch and go with that for now.
          Hide
          Leonardo Uribe added a comment -

          I checked it and the test Jakob provide works correctly. I commit a variant to check the case with redirect:

          return "viewparam.xhtml?faces-redirect=true&includeViewParams=true";

          It works, I don't see any problem. The idea is simple: just let the algorithm be executed on build view time form tags inside f:metadata, and handle f:event because it is an special case. Note Jakob provided a JUnit test for this one, so if there is something wrong, that test should fail and prevent myfaces from compile. It could be good if you can check if you are using the latest myfaces version from trunk.

          Show
          Leonardo Uribe added a comment - I checked it and the test Jakob provide works correctly. I commit a variant to check the case with redirect: return "viewparam.xhtml?faces-redirect=true&includeViewParams=true"; It works, I don't see any problem. The idea is simple: just let the algorithm be executed on build view time form tags inside f:metadata, and handle f:event because it is an special case. Note Jakob provided a JUnit test for this one, so if there is something wrong, that test should fail and prevent myfaces from compile. It could be good if you can check if you are using the latest myfaces version from trunk.
          Hide
          Mark Struberg added a comment -

          yup I always compile it myself. now I need to check why the test works and my real world app crashes...
          But it definitly crashes currently. Will give your new version a try now.

          Show
          Mark Struberg added a comment - yup I always compile it myself. now I need to check why the test works and my real world app crashes... But it definitly crashes currently. Will give your new version a try now.
          Hide
          Mark Struberg added a comment -

          Leo, I don't see any new commit still. Jakobs sample is a standalone webapp and not a junit test afaik... Please start it manually - txs.

          Show
          Mark Struberg added a comment - Leo, I don't see any new commit still. Jakobs sample is a standalone webapp and not a junit test afaik... Please start it manually - txs.
          Hide
          Mark Struberg added a comment -

          btw, the whole sample project is not really well placed under current20 (which is usually only a svn link ref collection) and the <repository> section to java.net should also be avoided

          Show
          Mark Struberg added a comment - btw, the whole sample project is not really well placed under current20 (which is usually only a svn link ref collection) and the <repository> section to java.net should also be avoided
          Hide
          Leonardo Uribe added a comment -

          The example is here:

          http://svn.apache.org/repos/asf/myfaces/current20/test-webapp/webapp/src/main/webapp/viewparam.xhtml

          The latest code has this code:

          <h:commandButton value="Postback" />
          <h:commandButton value="Postback Redirect" action="#

          {requestViewParamBean.doRedirect}

          " />

          Maybe I should say a "variant" of the example. The solution proposed is the same.

          Show
          Leonardo Uribe added a comment - The example is here: http://svn.apache.org/repos/asf/myfaces/current20/test-webapp/webapp/src/main/webapp/viewparam.xhtml The latest code has this code: <h:commandButton value="Postback" /> <h:commandButton value="Postback Redirect" action="# {requestViewParamBean.doRedirect} " /> Maybe I should say a "variant" of the example. The solution proposed is the same.
          Hide
          Mark Struberg added a comment -

          Leo, I've debugged through and it seems that ViewMetadataHandler#apply is not even called for my fragment.

          Please note that my xhtml consists of ui:composition and the action is in a ui:fragment. This might change the flow?

          Show
          Mark Struberg added a comment - Leo, I've debugged through and it seems that ViewMetadataHandler#apply is not even called for my fragment. Please note that my xhtml consists of ui:composition and the action is in a ui:fragment. This might change the flow?
          Hide
          Mark Struberg added a comment -

          Leo, it seems all work fine if the f:metadata and f:view (which I need to set the proper locale) is on the topmost layer in the view tree.

          The remaining question is if we should log a big fat warning if the f:view is somewhere deeper in the tree?

          Show
          Mark Struberg added a comment - Leo, it seems all work fine if the f:metadata and f:view (which I need to set the proper locale) is on the topmost layer in the view tree. The remaining question is if we should log a big fat warning if the f:view is somewhere deeper in the tree?
          Hide
          Leonardo Uribe added a comment -

          Ok, so we can close this issue. f:metadata should be on the topmost layer as says the facelets tld javadoc of f:metadata :

          "....Declare the metadata facet for this view. This must be a child of the <f:view>. This tag must reside within the top level XHTML file for the given viewId, or in a template client, but not in a template...".

          Right now if the component parent passed to f:metadata is not UIViewRoot, an exception is thrown, but this condition is only checked when viewMetadata is being built. I think we just check that condition always is enough. I'll close this issue as fixed.

          Show
          Leonardo Uribe added a comment - Ok, so we can close this issue. f:metadata should be on the topmost layer as says the facelets tld javadoc of f:metadata : "....Declare the metadata facet for this view. This must be a child of the <f:view>. This tag must reside within the top level XHTML file for the given viewId, or in a template client, but not in a template...". Right now if the component parent passed to f:metadata is not UIViewRoot, an exception is thrown, but this condition is only checked when viewMetadata is being built. I think we just check that condition always is enough. I'll close this issue as fixed.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development