MyFaces Core
  1. MyFaces Core
  2. MYFACES-2502

Component state is lost for composite component childs of facets relocated by composite:insertChildren or composite:insertFacet

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-beta
    • Fix Version/s: 2.0.0-beta-2
    • Component/s: JSR-314
    • Labels:
      None

      Description

      When partial state saving is not used, component state is lost for composite component childs of facets relocated by composite:insertChildren or composite:insertFacet

      To understand why this is happening, it is necessary to understand how facelets works in context and how composite:insertChildren and composite:insertFacet works, so I'll do a resume for it.

      In jsf 1.2, a facelet is applied in two cases:

      1. When a page is request for first time, the whole component tree is build.
      2. On a postback to update transient components like facelets UIInstruction.

      To see it in context, suppose a simple app that ask for a name an it print it on another component on the same page:

      <h:form>
      Name: <h:inputText value="

      {bean.name}/>
      Previous Name: <h:outputtext value="{bean.name}

      />
      <h:commandButton value="submit" action="submitToThisSamePage"/>
      </h:form>

      That is what happened when facelets + jsf 1.2 is used:

      First Request:

      • There is a call to FaceletViewHandler.buildView from FaceletViewHandler.renderView that cause the UIViewRoot instance to be filled for first time calling to f.apply().
      • The view is rendered.
      • Save the state for all non transient components found on the view. This include save the tree structure too, so it can be reconstructed later.

      Postback (The user send his name and do a submit):

      • Restore the tree structure and component state for all saved components.
      • All lifecycle phases continues until before renderView
      • On FaceletViewHandler.renderView there is a call to buildView, and this one causes all transient components like facelets UIInstruction to be added to the tree. ComponentHandler first try to detect if the component is on the view before create it, and if that is true do not create it, instead it takes this instance and continue apply the taghandles and it remove and add it from tree, to give the chance to other transient components to be created and added correctly.
      • The view is rendered.
      • Save the state for all non transient components found on the view. This include save the tree structure too, so it can be reconstructed later.

      In jsf 2.0 it happens something similar. ViewDeclarationLanguage and TagHandlerDelegate abstract classes were created, so some code was "relocated". To be clear, the algorithm in jsf 2.0 without partial state saving is this:

      First Request:

      • There is a call to ViewDeclarationLanguage.buildView from RenderResponseExecutor.execute that cause the UIViewRoot instance to be filled for first time calling to f.apply().
      • The view is rendered.
      • Save the state for all non transient components found on the view. This include save the tree structure too, so it can be reconstructed later.

      Postback (The user send his name and do a submit):

      • Restore the tree structure and component state for all saved components.
      • All lifecycle phases continues until before renderView
      • On RenderResponseExecutor.execute there is a call to buildView, and this one causes all transient components like facelets UIInstructions to be added to the tree. ComponentHandler first try to detect if the component is on the view before create it, and if that is true do not create it, instead it takes this instance and continue apply the taghandles and it remove and add it from tree, to give the chance to other transient components to be created and added correctly.
      • The view is rendered.
      • Save the state for all non transient components found on the view. This include save the tree structure too, so it can be reconstructed later.

      The algorithm in jsf 2.0 with partial state saving is different:

      First Request:

      • There is a call to ViewDeclarationLanguage.buildView from RenderResponseExecutor.execute that cause the UIViewRoot instance to be filled for first time calling to f.apply().
      • The view is rendered.
      • Save the state for all non transient components found on the view that has delta or was added after build. This does not include the structure, because all state is just saved on a big Map<String,Object> where the keys are the clientId for each component.

      Postback (The user send his name and do a submit):

      • StateManagementStrategy.restoreView calls ViewDeclarationLanguage.buildView passing an empty UIViewRoot to be filled for first time calling to f.apply().
      • Restore the component state for all saved components. Note the tree structure now is already provided.
      • All lifecycle phases continues until before renderView
      • On RenderResponseExecutor.execute there is a call to buildView, but this time, since the view is already filled, nothing happens. It just return.
      • The view is rendered.
      • Save the state for all non transient components found on the view that has delta or was added after build. This does not include the structure, because all state is just saved on a big Map<String,Object> where the keys are the clientId for each component.

      Really from a point of view, partial state saving integrates a param called FaceletViewHandler.buildBeforeRestore with other ideas found on Trinidad.

      Now suppose this scenario:

      Component impl:

      <composite:interface>
      </composite:interface>
      <composite:implementation>
      <h:outputText value=" Hello " />
      <p>
      <composite:insertChildren/>
      </p>
      <h:outputText value=" Bye " />
      </composite:implementation>

      Snippet on page:

      <testComposite:simpleInsertChildren id="sic">
      Mr.
      <h:inputText id="name" value="John " />
      Smith
      </testComposite:simpleInsertChildren>

      composite:insertChildren and composite:insertFacet uses a listener attached to PostAddToViewEvent to relocate the components. So, when the listener is called the body of testComposite:simpleInsertChildren looks like this:

      <h:outputText value=" Hello " />
      <p>
      Mr.
      <h:inputText id="name" value="John " />
      Smith
      </p>
      <h:outputText value=" Bye " />

      The reason why we do that on a listener is described on MYFACES-2317. In few words, we need the relocation occur from root to branches to allow nesting on composite components.

      Now the big question: Why the state is lost for child or facet components using composite:insertChildren or composite:insertFacet?

      When the component tree is updated (call to buildView on render response phase and postback), since the components inside the composite component were moved to some location "inside" the composite component, the current algorithm is unable to detect this condition, so the components are created again. The reason why there is no duplicate components in the view is facelets has an algoritm to mark and delete component instances that are not traversed (this is the real magic behind c:if tag). Then facelets detach and attach all components while traversing, triggering PostAddToViewEvent and our relocation listener is activated again (before do this the components on the location were deleted).

      Why this does not happens when partial state saving is used? In this case there is no "update" call. buildView just return because the view is already filled.
      In this case, c:if will not work as in jsf 1.2 if it depends on a value that changes on invoke application phase (MYFACES-2483).

      In this case we have two options:

      1. Do some algorithm that helps facelets to "jump" to the correct location where the component was relocated. Then on the listener, relocate the components created (UIInstructions instances) based on the component order saved on first time.

      Advantages: Keep things simple and only has to deal with the components that were moved in relocation.
      Disadvantages : c:if will not work correctly inside a composite component that uses composite:insertChildren or composite:insertFacet.

      2. Let facelets create the components and then on the listener "unify the tree", adding the new components and remove the ones that shouldn't be there.

      Advantages: c:if will work correctly on all situations.
      Disadvantages: The algorithm necessary has to traverse the tree for all children and facets looking for added components.

      I tried both solutions and finally I think solution 1 is the option to do. Obviously, things like this:

      <testComposite:simpleInsertChildren id="sic">
      Mr.
      <c:if test="#

      {helloWorldIfBean.toogle}">
      <p>Pepito</p>
      <h:outputText value="Perez"/>
      </c:if>
      Smith
      </testComposite:simpleInsertChildren>

      will not work correctly, but if you put this one like this:

      <testComposite:simpleInsertChildren id="sic">

      Mr.
      <h:panelGroup>
      <c:if test="#{helloWorldIfBean.toogle}

      ">
      <p>Pepito</p>
      <h:outputText value="Perez"/>
      </c:if>
      </h:panelGroup>
      Smith
      </testComposite:simpleInsertChildren>

      It will.

      If no objections I'll commit the solution 1 soon.

      1. MYFACES-2502-4.patch
        46 kB
        Leonardo Uribe
      2. MYFACES-2502-3.patch
        39 kB
        Leonardo Uribe
      3. MYFACES-2502-2.patch
        31 kB
        Leonardo Uribe

        Issue Links

          Activity

          Hide
          Leonardo Uribe added a comment -

          I attached another patch ( MYFACES-2502-3.patch ) that does not have the problem of c:if inside a composite component. The solution was move the ordering code to ComponentTagHandlerDelegate and UIInstructionHandler, because this is the only point where we can keep track of the components that needs to be added/preserved/removed.

          Show
          Leonardo Uribe added a comment - I attached another patch ( MYFACES-2502 -3.patch ) that does not have the problem of c:if inside a composite component. The solution was move the ordering code to ComponentTagHandlerDelegate and UIInstructionHandler, because this is the only point where we can keep track of the components that needs to be added/preserved/removed.
          Hide
          Jakob Korherr added a comment -

          so with MYFACES-2502-3.patch the problem with c:if in the following scenario is gone?

          <testComposite:simpleInsertChildren id="sic">
          Mr.
          <c:if test="#

          {helloWorldIfBean.toogle}

          ">
          <p>Pepito</p>
          <h:outputText value="Perez"/>
          </c:if>
          Smith
          </testComposite:simpleInsertChildren>

          If not, maybe we could just put all top-level c:if tags inside a composite component automatically in a h:panelGroup.

          Show
          Jakob Korherr added a comment - so with MYFACES-2502 -3.patch the problem with c:if in the following scenario is gone? <testComposite:simpleInsertChildren id="sic"> Mr. <c:if test="# {helloWorldIfBean.toogle} "> <p>Pepito</p> <h:outputText value="Perez"/> </c:if> Smith </testComposite:simpleInsertChildren> If not, maybe we could just put all top-level c:if tags inside a composite component automatically in a h:panelGroup.
          Hide
          Leonardo Uribe added a comment -

          Yes, with the latest patch the problem is no more present. I think is the best we can do for solve this issue (really I spent a lot of time finding it and solving it).

          Suggestions are welcome.

          Show
          Leonardo Uribe added a comment - Yes, with the latest patch the problem is no more present. I think is the best we can do for solve this issue (really I spent a lot of time finding it and solving it). Suggestions are welcome.
          Hide
          Jakob Korherr added a comment -

          ok, looks good to me!

          Show
          Jakob Korherr added a comment - ok, looks good to me!
          Hide
          Leonardo Uribe added a comment -

          This is the patch to be committed. It has some enhancements like cache some build time params on FaceletContext. I just let it here as reference

          Show
          Leonardo Uribe added a comment - This is the patch to be committed. It has some enhancements like cache some build time params on FaceletContext. I just let it here as reference

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development