ICEfaces
  1. ICEfaces
  2. ICE-6643

WindowAndViewIDSetup and FormSubmit: Event handlers cause duplicate components

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 2.0.1
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      JBoss 5.1
      JSF 2.1
      Portlet-Faces-Bridge 2.0.0 BETA4
    • Affects:
      Sample App./Tutorial
    • Workaround Exists:
      Yes
    • Workaround Description:
      Hide
      Please add a

      if (event instanceof PostAddToViewEvent) {
      <original source>
      }

      to the "processEvent(SystemEvent event)"- Method.
      Show
      Please add a if (event instanceof PostAddToViewEvent) { <original source> } to the "processEvent(SystemEvent event)"- Method.
    • Community Contribution:
      Yes

      Description

      While executing an AJAX-Request with a full render (by navigation-case), the components in the event handlers are to be added a second time.
      This results from the Listener to handle the event twice.
      1. ICE-6643-2.patch
        3 kB
        Deryk Sinotte
      2. Patch - Icefaces_Events.patch
        14 kB
        Christian Heike
      3. Patch - Icefaces_Events.patch
        13 kB
        Christian Heike

        Activity

        Christian Heike created issue -
        Hide
        Christian Heike added a comment -

        Solution for the problem.

        Show
        Christian Heike added a comment - Solution for the problem.
        Christian Heike made changes -
        Field Original Value New Value
        Attachment Patch - Icefaces_Events.patch [ 12929 ]
        Hide
        Christian Heike added a comment -

        Please ignore the first patch.
        This actually works, though it is just a workaround, I still do not know why the event listener is called twice.
        I guess it is caused by revisiting the DOM for the changes, but I am not sure.
        Maybe you can bring light into this.

        Show
        Christian Heike added a comment - Please ignore the first patch. This actually works, though it is just a workaround, I still do not know why the event listener is called twice. I guess it is caused by revisiting the DOM for the changes, but I am not sure. Maybe you can bring light into this.
        Christian Heike made changes -
        Attachment Patch - Icefaces_Events.patch [ 12930 ]
        Christian Heike made changes -
        Attachment Patch - Icefaces_Events.patch [ 12930 ]
        Hide
        Christian Heike added a comment -

        The patch submitted before did not work in all cases, here is a solution which fits in better.

        Show
        Christian Heike added a comment - The patch submitted before did not work in all cases, here is a solution which fits in better.
        Christian Heike made changes -
        Attachment Patch - Icefaces_Events.patch [ 12931 ]
        Ken Fyten made changes -
        Salesforce Case []
        Fix Version/s 2.0.1 [ 10255 ]
        Assignee Priority P2
        Assignee Ted Goddard [ ted.goddard ]
        Hide
        Ted Goddard added a comment -

        If multiple copies of icefaces.jar are present in the web application, these event listeners will be called twice. Is this the case?

        How did you detect that components are duplicated and what negative effect does this have on the application?

        Show
        Ted Goddard added a comment - If multiple copies of icefaces.jar are present in the web application, these event listeners will be called twice. Is this the case? How did you detect that components are duplicated and what negative effect does this have on the application?
        Hide
        Christian Heike added a comment -

        It is not the case.I have reviewed the issue into a certain depth and found the listener to be invoked in both, RestoreViewPhase and RenderPhase.
        This only happens when PartialViewContext.isRenderAll is true.
        It also could point to a problem with the bridge. Please also note the issue ICE-6644 I have created. Thank you.

        Show
        Christian Heike added a comment - It is not the case.I have reviewed the issue into a certain depth and found the listener to be invoked in both, RestoreViewPhase and RenderPhase. This only happens when PartialViewContext.isRenderAll is true. It also could point to a problem with the bridge. Please also note the issue ICE-6644 I have created. Thank you.
        Hide
        Ted Goddard added a comment -

        Are you able to observe this in any of the sample application provided with ICEfaces 2? If so, include steps to reproduce and observe the problem. The other JIRA mentions a portlet use case. Do you observe this problem with both portlets and servlets?

        Show
        Ted Goddard added a comment - Are you able to observe this in any of the sample application provided with ICEfaces 2? If so, include steps to reproduce and observe the problem. The other JIRA mentions a portlet use case. Do you observe this problem with both portlets and servlets?
        Hide
        Deryk Sinotte added a comment -

        Modified patch to remove whitespace diff so it's easier to evaluate the impact of the relevant changes.

        Show
        Deryk Sinotte added a comment - Modified patch to remove whitespace diff so it's easier to evaluate the impact of the relevant changes.
        Deryk Sinotte made changes -
        Attachment ICE-6643-2.patch [ 12932 ]
        Hide
        Deryk Sinotte added a comment -

        Mircea, please evaluate the impact of the ICE-6433-2 patch on the current ICEfaces 2 trunk code. The patch has been submitted by a customer in related to doing Ajax navigation in a portal.

        Christian, can you please provide a simple test case that we can use to verify the problem and the fix and that we could potentially add to our regression test suite.

        Show
        Deryk Sinotte added a comment - Mircea, please evaluate the impact of the ICE-6433-2 patch on the current ICEfaces 2 trunk code. The patch has been submitted by a customer in related to doing Ajax navigation in a portal. Christian, can you please provide a simple test case that we can use to verify the problem and the fix and that we could potentially add to our regression test suite.
        Deryk Sinotte made changes -
        Salesforce Case []
        Community Contribution [Yes]
        Assignee Ted Goddard [ ted.goddard ] Mircea Toma [ mircea.toma ]
        Hide
        Mircea Toma added a comment -

        I used a test application that has both redirecting and forwarding navigation rules. In both cases the system event handler is invoked only once, right after a new HtmlForm component is added to the component tree.

        Of course during navigation the JSF lifecycle is ran twice, each time with a newly created component tree. The first lifecycle is triggered by the postback that initiated the navigation. The second lifecycle corresponds to the view defined by the navigation case.

        Until I see a test case that contradicts these finding I'm marking this issue as "Cannot Reproduce".

        Show
        Mircea Toma added a comment - I used a test application that has both redirecting and forwarding navigation rules. In both cases the system event handler is invoked only once, right after a new HtmlForm component is added to the component tree. Of course during navigation the JSF lifecycle is ran twice, each time with a newly created component tree. The first lifecycle is triggered by the postback that initiated the navigation. The second lifecycle corresponds to the view defined by the navigation case. Until I see a test case that contradicts these finding I'm marking this issue as "Cannot Reproduce".
        Mircea Toma made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Cannot Reproduce [ 5 ]
        Hide
        Mircea Toma added a comment -

        Need to test in Liferay.

        Show
        Mircea Toma added a comment - Need to test in Liferay.
        Mircea Toma made changes -
        Resolution Cannot Reproduce [ 5 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Christian Heike added a comment -

        Sorry it took me so long, I had to simplify my portlet-application.
        I did not include my ant-build file since it is very special, but you should be able to compile the srcs and then do a war (with WebContent to the root and the compiled classes to "WEB-INF/classes").

        I hope you can reproduce the problem now.

        Christian

        Show
        Christian Heike added a comment - Sorry it took me so long, I had to simplify my portlet-application. I did not include my ant-build file since it is very special, but you should be able to compile the srcs and then do a war (with WebContent to the root and the compiled classes to "WEB-INF/classes"). I hope you can reproduce the problem now. Christian
        Christian Heike made changes -
        Attachment AJAXTest-ViewRebuilt.zip [ 12964 ]
        Ken Fyten made changes -
        Salesforce Case []
        Assignee Priority P2 P1
        Assignee Mircea Toma [ mircea.toma ] Deryk Sinotte [ deryk.sinotte ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24186 Thu Mar 17 11:26:27 MDT 2011 deryk.sinotte ICE-6643: PostAddToViewEvent adding components multiple times, check to ensure we only do it in render phase
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/FormSubmit.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/WindowAndViewIDSetup.java
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24187 Thu Mar 17 11:46:51 MDT 2011 deryk.sinotte ICE-6643: reverting fix as incomplete because sometimes only RESTORE_VIEW is executed with the PostAddToViewEvent
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/FormSubmit.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/WindowAndViewIDSetup.java
        Hide
        Deryk Sinotte added a comment -

        It turns out that, under certain situations related to dynamic includes and composite components, the PostAddToViewEvent can be called multiple times (RESTORE_VIEW, RENDER_RESPONSE) during the same JSF lifecycle. Since we only care about adding the components from WindowAndViewIDSetup and FormSubmit during the render phase, I've added a check for the current phase and bail out if it's not the render phase.

        With this fix and the related navigation fixes in the latest version of the bridge, the supplied test case now works as originally designed. Resolving as fixed.

        Show
        Deryk Sinotte added a comment - It turns out that, under certain situations related to dynamic includes and composite components, the PostAddToViewEvent can be called multiple times (RESTORE_VIEW, RENDER_RESPONSE) during the same JSF lifecycle. Since we only care about adding the components from WindowAndViewIDSetup and FormSubmit during the render phase, I've added a check for the current phase and bail out if it's not the render phase. With this fix and the related navigation fixes in the latest version of the bridge, the supplied test case now works as originally designed. Resolving as fixed.
        Deryk Sinotte made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24188 Thu Mar 17 12:53:14 MDT 2011 deryk.sinotte ICE-6643: previous failures may have been related to JavaScript minification so putting the original fix back in
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/FormSubmit.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/WindowAndViewIDSetup.java
        Hide
        Deryk Sinotte added a comment -

        Re-opening the case for further investigation.

        It might be worthwhile to test a non-portlet app with composite components and non-redirect navigation.

        Show
        Deryk Sinotte added a comment - Re-opening the case for further investigation. It might be worthwhile to test a non-portlet app with composite components and non-redirect navigation.
        Deryk Sinotte made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Deryk Sinotte added a comment -

        I had added in this guard to ensure it only ran in one phase:

        if( context.getCurrentPhaseId() != PhaseId.RENDER_RESPONSE )

        { //ICE-6643: In certain circumstances, it's possible that the PostAddToViewEvent is // triggered more than once in the same lifecycle. We only want it added // during the render phase. return; }

        Which worked splendidly to fix the supplied test case. Further testing in other behaviours was not so joyous. It turns out that the PostAddToViewEvent is sometimes fired:

        • only during RENDER_RESPONSE
        • only during RESTORE_VIEW
        • during both phases

        So I reverted the fix back out again and will continue to investigate.

        Show
        Deryk Sinotte added a comment - I had added in this guard to ensure it only ran in one phase: if( context.getCurrentPhaseId() != PhaseId.RENDER_RESPONSE ) { //ICE-6643: In certain circumstances, it's possible that the PostAddToViewEvent is // triggered more than once in the same lifecycle. We only want it added // during the render phase. return; } Which worked splendidly to fix the supplied test case. Further testing in other behaviours was not so joyous. It turns out that the PostAddToViewEvent is sometimes fired: only during RENDER_RESPONSE only during RESTORE_VIEW during both phases So I reverted the fix back out again and will continue to investigate.
        Hide
        Deryk Sinotte added a comment -

        The previous failure appears to have been related to a different issue (the minification problems of the Compat related JavaScript libs). It would work to load the portlet and allow one click to succeed and add a new panel but further button actions did not work.

        I reverted everything to the latest trunks (icefaces and portletfaces), rebuilt everything, and retested with the phase-checking reinstated back into WindowAndViewIDSetup and FormSubmit classes. Both test cases worked (the portlet navigation and the 'required' validation).

        Show
        Deryk Sinotte added a comment - The previous failure appears to have been related to a different issue (the minification problems of the Compat related JavaScript libs). It would work to load the portlet and allow one click to succeed and add a new panel but further button actions did not work. I reverted everything to the latest trunks (icefaces and portletfaces), rebuilt everything, and retested with the phase-checking reinstated back into WindowAndViewIDSetup and FormSubmit classes. Both test cases worked (the portlet navigation and the 'required' validation).
        Deryk Sinotte made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24200 Fri Mar 18 14:49:32 MDT 2011 ted.goddard reverting due to regression (ICE-6643)
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/FormSubmit.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/WindowAndViewIDSetup.java
        Hide
        Ken Fyten added a comment -

        This commit is causing new regression failures with:

        http://10.18.39.129:8080/ICE-4090 (drop down list not working the first time)
        http://10.18.39.129:8080/ICE-3618 (reset button not working)
        http://10.18.39.129:8080/ICE-4158 (drop down list not working the first time)

        Ted's analysis:

        The following is missing on the first page load causing a
        full <form> update:

        <script id="iceForm:iceForm_captureSubmit" type="text/javascript">ice.captureSubmit('iceForm',false);ice.captureEnterKey('iceForm');</script>

        Show
        Ken Fyten added a comment - This commit is causing new regression failures with: http://10.18.39.129:8080/ICE-4090 (drop down list not working the first time) http://10.18.39.129:8080/ICE-3618 (reset button not working) http://10.18.39.129:8080/ICE-4158 (drop down list not working the first time) Ted's analysis: The following is missing on the first page load causing a full <form> update: <script id="iceForm:iceForm_captureSubmit" type="text/javascript">ice.captureSubmit('iceForm',false);ice.captureEnterKey('iceForm');</script>
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Assignee Deryk Sinotte [ deryk.sinotte ] Ted Goddard [ ted.goddard ]
        Hide
        Ted Goddard added a comment -

        Confirmed ICE-4090 is resolved when the SystemEventListener s are reverted.

        The earlier analysis was slightly reversed: the ice.captureSubmit() JavaScript is actually missing from Ajax update not the initial page render.

        Show
        Ted Goddard added a comment - Confirmed ICE-4090 is resolved when the SystemEventListener s are reverted. The earlier analysis was slightly reversed: the ice.captureSubmit() JavaScript is actually missing from Ajax update not the initial page render.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24202 Fri Mar 18 16:04:22 MDT 2011 ted.goddard added facesContext guard attribute against duplicates (ICE-6643)
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/FormSubmit.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/event/WindowAndViewIDSetup.java
        Hide
        Ted Goddard added a comment -

        Porting the sample to a Servlet test case shows the following in the log when clicking on "Greetings":

        SEVERE: JSF1007: Duplicate component ID j_idt7:menuForm:menuForm_windowviewid found in view.

        Show
        Ted Goddard added a comment - Porting the sample to a Servlet test case shows the following in the log when clicking on "Greetings": SEVERE: JSF1007: Duplicate component ID j_idt7:menuForm:menuForm_windowviewid found in view.
        Ted Goddard made changes -
        Attachment test.zip [ 12985 ]
        Hide
        Ted Goddard added a comment -

        Attached test.zip contains files to be added to component-showcase to reproduce both ICE-4090 and this issue in a Servlet application.

        Show
        Ted Goddard added a comment - Attached test.zip contains files to be added to component-showcase to reproduce both ICE-4090 and this issue in a Servlet application.
        Hide
        Ted Goddard added a comment -

        The most recent fix uses the following technique:

        + //guard against duplicates within the same JSF lifecycle
        + if (null != context.getAttributes().get(componentId))

        { + return; + }

        + context.getAttributes().put(componentId, componentId);
        +

        It was observed that the duplicate components were added during the same lifecycle, so the above check ensures that the component is added only once per FacesContext instance. It is possible that this technique makes some of the other duplicate-checking obsolete, however, this may not be the case since other cases of duplication result from differences in event handling after state saving (this is why the use of component attribute markers is dangerous since the markers may be state-saved with the component even though the transient component is not saved).

        Show
        Ted Goddard added a comment - The most recent fix uses the following technique: + //guard against duplicates within the same JSF lifecycle + if (null != context.getAttributes().get(componentId)) { + return; + } + context.getAttributes().put(componentId, componentId); + It was observed that the duplicate components were added during the same lifecycle, so the above check ensures that the component is added only once per FacesContext instance. It is possible that this technique makes some of the other duplicate-checking obsolete, however, this may not be the case since other cases of duplication result from differences in event handling after state saving (this is why the use of component attribute markers is dangerous since the markers may be state-saved with the component even though the transient component is not saved).
        Hide
        Ted Goddard added a comment -

        Please test the latest trunk in the portal environment that previously reproduced the problem. (It has also been reproduced with a Servlet, but the fix needs verification in a Portlet environment.)

        Show
        Ted Goddard added a comment - Please test the latest trunk in the portal environment that previously reproduced the problem. (It has also been reproduced with a Servlet, but the fix needs verification in a Portlet environment.)
        Ted Goddard made changes -
        Assignee Ted Goddard [ ted.goddard ] Deryk Sinotte [ deryk.sinotte ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24213 Mon Mar 21 16:06:28 MDT 2011 deryk.sinotte ICE-6643: adjusted PortletUtil to use simpler code due to changes in PortletFaces bridge
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/samples/compat/component-showcase-portlets/src/main/java/org/icefaces/application/showcase/util/PortletUtil.java
        Hide
        Deryk Sinotte added a comment -

        So there were changes to the following areas that ended up requiring some effort to ensure everything was stable:

        • ICEfaces core
        • ICEfaces components
        • JSF libs
        • PortletFaces bridge

        I updated to the latest code in all the affected areas and modified the examples based on these new libraries. After stabilizing things and re-testing, I think we're back to a point where the attached test case as well as the modified component showcase are both behaving as desired so I'm marking as fixed.

        Neil was kind enough to make a change that should benefit developers looking to utilise init parameters in both the portlet.xml and web.xml files.

        Show
        Deryk Sinotte added a comment - So there were changes to the following areas that ended up requiring some effort to ensure everything was stable: ICEfaces core ICEfaces components JSF libs PortletFaces bridge I updated to the latest code in all the affected areas and modified the examples based on these new libraries. After stabilizing things and re-testing, I think we're back to a point where the attached test case as well as the modified component showcase are both behaving as desired so I'm marking as fixed. Neil was kind enough to make a change that should benefit developers looking to utilise init parameters in both the portlet.xml and web.xml files.
        Deryk Sinotte made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Affects [Sample App./Tutorial]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #24228 Wed Mar 23 15:35:09 MDT 2011 deryk.sinotte ICE-6643: made same adjustments to the PortletUtil of ACE showcase as was done to compat Component Showcase due to changes in PortletFaces bridge
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/samples/compat/component-showcase-portlets/src/main/java/org/icefaces/application/showcase/util/PortletUtil.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/samples/ace/showcase-portlet/src/main/java/org/icefaces/portlet/PortletUtil.java
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Assignee Priority P1

          People

          • Assignee:
            Deryk Sinotte
            Reporter:
            Christian Heike
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: