ICEfaces
  1. ICEfaces
  2. ICE-7755

ace:dialog unwrapped script tag can cause larger than necessary page updates

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 3.0.1, EE-3.0.0.GA
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      IF3.x

      Description

      ace:dialog has unwrapped script tag causing a form-wide update in the update following opening, causing long running updates in IE8 in particular.

        Activity

        Hide
        Nils Lundquist added a comment -

        r27758 - Wrapped script tag in id'd span.

        Show
        Nils Lundquist added a comment - r27758 - Wrapped script tag in id'd span.
        Hide
        Ken Fyten added a comment -

        This change is resulting in regression failures when trying to dynamically change attribute values:

        Icefaces3.0.x-maintenance revision# 27807
        Server: tomcat6
        Browser: FF3.6, IE8, GoogleChrome16

        New failures: All browsers: dialog: Dynamic Attribute test is failing (Fails manually, the header cannot be displayed dynamically after the dialog is rendered. Further testing shows that this is true for other attributes as well (closable, resizable)).

        Show
        Ken Fyten added a comment - This change is resulting in regression failures when trying to dynamically change attribute values: Icefaces3.0.x-maintenance revision# 27807 Server: tomcat6 Browser: FF3.6, IE8, GoogleChrome16 New failures: All browsers: dialog: Dynamic Attribute test is failing (Fails manually, the header cannot be displayed dynamically after the dialog is rendered. Further testing shows that this is true for other attributes as well (closable, resizable)).
        Hide
        Ted Goddard added a comment -

        The customer test case that caused the unexpected page update was not fully investigated. It's possible that any change to the ace:dialog script should force the containing dialog to be updated, in which case this change is not good. Further investigation of the original test case is still necessary.

        Show
        Ted Goddard added a comment - The customer test case that caused the unexpected page update was not fully investigated. It's possible that any change to the ace:dialog script should force the containing dialog to be updated, in which case this change is not good. Further investigation of the original test case is still necessary.
        Hide
        Ken Fyten added a comment -

        I have reverted this change on the icefaces3/trunk and icefaces-3.0.x-maintenance branch.

        Show
        Ken Fyten added a comment - I have reverted this change on the icefaces3/trunk and icefaces-3.0.x-maintenance branch.
        Hide
        Arturo Zambrano added a comment -

        Fixed at revision 28087 in the trunk and at revision 28088 in the maintenance branch.

        The problem was that the whole containing form was being updated since the script tag didn't have an id, so if the form was big and/or if it took some time to initialize, that same amount of time was being consumed at every dynamic update.

        Therefore, both the markup and the script tag were wrapped inside a container div with the id 'clientId + "_container"'. This was tested with the QA test app, and this fix didn't cause regressions with dynamic attributes. The approach described in the comments above was also tried at some point, but it did cause regressions with dynamic attributes.It is unclear why this happens. Another approach that was tried was placing the script tag inside the main dialog div, but the same problem was observed. At the end, the only thing that worked was to create a container div that wraps both the original dialog markup and the script tag.

        Show
        Arturo Zambrano added a comment - Fixed at revision 28087 in the trunk and at revision 28088 in the maintenance branch. The problem was that the whole containing form was being updated since the script tag didn't have an id, so if the form was big and/or if it took some time to initialize, that same amount of time was being consumed at every dynamic update. Therefore, both the markup and the script tag were wrapped inside a container div with the id 'clientId + "_container"'. This was tested with the QA test app, and this fix didn't cause regressions with dynamic attributes. The approach described in the comments above was also tried at some point, but it did cause regressions with dynamic attributes.It is unclear why this happens. Another approach that was tried was placing the script tag inside the main dialog div, but the same problem was observed. At the end, the only thing that worked was to create a container div that wraps both the original dialog markup and the script tag.
        Hide
        Arturo Zambrano added a comment -

        Re-opened.

        If a subtree render is invoked via f:ajax, the DOM subtree will be looked up by client ID, so the clientID on the containing element of the component must be exactly the client ID of the component.

        Show
        Arturo Zambrano added a comment - Re-opened. If a subtree render is invoked via f:ajax, the DOM subtree will be looked up by client ID, so the clientID on the containing element of the component must be exactly the client ID of the component.
        Hide
        Arturo Zambrano added a comment -

        Changed id of wrapping div to

        {clientId} and that of the div containing the main markup to {clientId}

        + "_main". Committed at revision 28125 in the trunk and at revision 28126 in the maintenance branch.

        Show
        Arturo Zambrano added a comment - Changed id of wrapping div to {clientId} and that of the div containing the main markup to {clientId} + "_main". Committed at revision 28125 in the trunk and at revision 28126 in the maintenance branch.

          People

          • Assignee:
            Arturo Zambrano
            Reporter:
            Nils Lundquist
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: