Tapestry
  1. Tapestry
  2. TAPESTRY-1023

Calling updateComponent in @EventListener method on a component that is prerendered results in null update

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1
    • Fix Version/s: 4.1.1
    • Component/s: Annotations
    • Labels:
      None
    • Environment:
      Tapestery 4.1 Head, JBoss 4.0.4 GA, Windows XP

      Description

      I had a problem with @EventListener tonight but I'm not sure if this is a
      bug or ignorance on my part.
      (I am working with the latest 4.1 version from svn)

      First let me explain what I am trying to do....
      I have a form with several PropertySelection input fields on it. What I
      want to do is change the selectable options based on what has already been
      selected.
      e.g. Say the form has two select fields A & B
      A has options 1,2,3.
      If the value of A is 1, then B should have options 11, 12, 13
      If the value of A is 2, then B should have options 21, 22, 23
      If the value of A is 3, then B should have options 31, 32, 33

      Now in the page template the fields are defined as follows

      <div class="field">
      <label jwcid="@FieldLabel" field="component:A">Role</label>
      <input jwcid="A@PropertySelection" value="ognl:A" displayName="A"
      model="ognl:aModel"/>
      </div>
      <div class="field">
      <label jwcid="@FieldLabel" field="component:B">B</label>
      <input jwcid="B@PropertySelection" value="ognl:B" displayName="B"
      model="ognl:bModel"/>
      </div>

      To update the options displayed by B I defined the following method

      @EventListener(targets="A", events="onchange", submitForm ="form")
      public void selectA(IRequestCycle cycle) {
      bModel = .... New model for b based on value of A ....
      cycle.getResponseBuilder().updateComponent("b");
      }

      What I was expecting to happen was that when the value of A was changed then
      B would be rerendered with the new options based on A's value.
      What actually happened was that as soon as I changed the value of A then B
      was rendered with no options at all.

      After a bit of debugging I eventually worked out that what was happening
      was:

      • A's value was changed & the event listener was invoked
      • The model for B was updated correctly
      • The FieldLabel that refered to B was rendered with a NullWriter
      • this field label prerendered B (to get its id) with the NullWriter
      • B was rendered with a real writer but didn't output anything because it
        had been prerendered
      • The page was updated an empty B element (cause the NullWriter discards
        everything).

      To work around this I gave the FieldLabel (for B) an explicit id and added
      an updateComponent call for the this id. When I did this the page behaved
      as I was expecting.

      Now my question: Is this behaviour correct?
      Or have I approached the problem incorrectly?

      It seems a bit unintuitive, I would prefer B to render properly when I asked
      it to be updated, rather than having to update both it and the label.

      However I'm not sure how to get my desired behaviour. Maybe the
      DojoAjaxResponseBuilder could check the form prerender map after each
      component render and clear out any prerender's with null results....
      ...or maybe wiser heads can come up with a much more elegant solution

      1. TAP-1023.patch
        36 kB
        Ben Sommerville
      2. TAP-1023.patch
        37 kB
        Ben Sommerville

        Activity

        Hide
        Ben Sommerville added a comment -

        I thought about this a bit more overnight & decided that my suggested solution probably won't work. Removing the component from the prerender map will result in the component rendering twice. I'm pretty sure this is not a good thing & could lead to some strange bugs .

        Ideally when prerendering a component that is being updated we would use a real writer, rather than the NullWriter. But I cant see how to accomplish this without rendering all components with a real writer (or significantly changing the writer interface)

        Show
        Ben Sommerville added a comment - I thought about this a bit more overnight & decided that my suggested solution probably won't work. Removing the component from the prerender map will result in the component rendering twice. I'm pretty sure this is not a good thing & could lead to some strange bugs . Ideally when prerendering a component that is being updated we would use a real writer, rather than the NullWriter. But I cant see how to accomplish this without rendering all components with a real writer (or significantly changing the writer interface)
        Hide
        Ben Sommerville added a comment -

        Digging into prerendering a bit more, I think I found another bug (haven't confirmed it with a test tho).

        If a FieldLabel is after its field & has prerender set to false, then it will end up with a null (or incorrect) "for" attribute. This is because the "for" attribute is set from its field.clientId. When the field is rendered first, its clientId is set to null once it has rendered (AbstractComponent.render). Then when the FieldLabel goes to get the client id it is no longer available.

        Show
        Ben Sommerville added a comment - Digging into prerendering a bit more, I think I found another bug (haven't confirmed it with a test tho). If a FieldLabel is after its field & has prerender set to false, then it will end up with a null (or incorrect) "for" attribute. This is because the "for" attribute is set from its field.clientId. When the field is rendered first, its clientId is set to null once it has rendered (AbstractComponent.render). Then when the FieldLabel goes to get the client id it is no longer available.
        Hide
        Ben Sommerville added a comment -

        I think I have a solution.

        It appears to me that the only component that uses (invokes) prerendering is the FieldLabel. The only reason it prerenders a field is to obtain its id (on the client). With the other changes in 4.1 around the generation (& caching) of client id, I'd say a better approach would be to ditch the prerendering functionality altogether & just allow the FieldLabel component to obtain the client id of its field.

        This has the advantage of simplifying components (they no longer need to worry about prerendering) & makes everthing work with theDojoResponseBuilder.

        Show
        Ben Sommerville added a comment - I think I have a solution. It appears to me that the only component that uses (invokes) prerendering is the FieldLabel. The only reason it prerenders a field is to obtain its id (on the client). With the other changes in 4.1 around the generation (& caching) of client id, I'd say a better approach would be to ditch the prerendering functionality altogether & just allow the FieldLabel component to obtain the client id of its field. This has the advantage of simplifying components (they no longer need to worry about prerendering) & makes everthing work with theDojoResponseBuilder.
        Hide
        Ben Sommerville added a comment -

        Patch to remove prerender functions and substitute ability to allocated client id of component before it is rendered. This removes the problem of the Ajax Response builder not rendering components refered to by FieldLabels.

        FieldLabel still needs "prerender" attribute to work out whether it is before its field & thus needs to allocate the client id.

        Hmm, I just realised that FieldLabel will still have a problem if it is after its field. The client id will be cleared after the field renders & FieldLable will no longer have access.

        However this aside, the patch should solve the main problem.

        Show
        Ben Sommerville added a comment - Patch to remove prerender functions and substitute ability to allocated client id of component before it is rendered. This removes the problem of the Ajax Response builder not rendering components refered to by FieldLabels. FieldLabel still needs "prerender" attribute to work out whether it is before its field & thus needs to allocate the client id. Hmm, I just realised that FieldLabel will still have a problem if it is after its field. The client id will be cleared after the field renders & FieldLable will no longer have access. However this aside, the patch should solve the main problem.
        Hide
        Ben Sommerville added a comment -

        Slightly improved patch.
        This handles getting the clientId of a component after it has been rendered.

        Show
        Ben Sommerville added a comment - Slightly improved patch. This handles getting the clientId of a component after it has been rendered.
        Hide
        Jesse Kuhnert added a comment -

        Thanks for all your hard work on this one Ben. It's fixed now.

        Show
        Jesse Kuhnert added a comment - Thanks for all your hard work on this one Ben. It's fixed now.

          People

          • Assignee:
            Jesse Kuhnert
            Reporter:
            Ben Sommerville
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development