ICEfaces
  1. ICEfaces
  2. ICE-9995

JavaScript error in notifyAllOnElementUpdateCallbacks function

    Details

    • Assignee Priority:
      P1
    • Support Case References:
    • Workaround Exists:
      Yes
    • Workaround Description:
      Hide
      Setting the following context param helps the issue but at the risk of performance:

      <context-param>
      <param-name>org.icefaces.clientSideElementUpdateDetermination</param-name>
      <param-value>true</param-value>
      </context-param>
      Show
      Setting the following context param helps the issue but at the risk of performance: <context-param> <param-name>org.icefaces.clientSideElementUpdateDetermination</param-name> <param-value>true</param-value> </context-param>

      Description

      The following issue has been seen by one of our customers:

      "one of our pages is getting javascript error Cannot read property 'data-onElementUpdate' on undefined during an ajax request. I have debugged the page and found the problem is in namespace.notifyAllOnElementUpdateCallbacks function. The error is caused in the loop within this function because the ice.onElementUpdate function for the <ace:dateTimeEntry> component which is on this page removes the dateTimeEntry input field and associated calendar image from the DOM which also removes two tags from the elements array within notifyAllOnElementUpdateCallbacks. When the index var i of the for loop gets to the second from last entry, then the var e will be undefined (since max loop index is based on original size of elements array, defined in var l, but size of elements is now reduced by 2)."

        Activity

        Hide
        Arturo Zambrano added a comment -

        Committed fix to the 4.0 trunk at revision 40899 and to the 3.3 EE maintenance branch at revision 40900.

        Even though I couldn't find a way to reproduce this issue, I could see that this situation is likely to happen, not only with the ace:dateTimeEntry component but with any component. The problem was that notifyAllOnElementUpdateCallbacks() obtains an array of all elements on the page (by calling document.body.getElementsByTagName('*')), and then uses a for-loop to process each of these elements, looking for a 'data-onElementUpdate' attribute. If it exists, it executes its contents as a callback function. Now, while processing all elements on the page in this way, we weren't making sure that each element still existed in the DOM. It is natural to expect that these onElementUpdate callbacks will remove elements from the DOM to clean up, so in some cases, some nodes were deleted by these callback functions before they got to be processed in this for-loop. Time-out functions and the amount of rendered HTML markup on the page could also be factors in this issue.

        The fix for this issue wasn't in the component code, as the onElementUpdate callback was simply doing what it was meant to do: clean up. Moreover, other components could have produced the same issue. So, as the customer suggested, the fix consists in adding conditionals to notifyAllOnElementUpdateCallbacks() (in the bridge code) to ensure that the element being processed still exists. We were already checking for the existence of the callback function before invoking it, so this is just another standard check before that.

        Show
        Arturo Zambrano added a comment - Committed fix to the 4.0 trunk at revision 40899 and to the 3.3 EE maintenance branch at revision 40900. Even though I couldn't find a way to reproduce this issue, I could see that this situation is likely to happen, not only with the ace:dateTimeEntry component but with any component. The problem was that notifyAllOnElementUpdateCallbacks() obtains an array of all elements on the page (by calling document.body.getElementsByTagName('*')), and then uses a for-loop to process each of these elements, looking for a 'data-onElementUpdate' attribute. If it exists, it executes its contents as a callback function. Now, while processing all elements on the page in this way, we weren't making sure that each element still existed in the DOM. It is natural to expect that these onElementUpdate callbacks will remove elements from the DOM to clean up, so in some cases, some nodes were deleted by these callback functions before they got to be processed in this for-loop. Time-out functions and the amount of rendered HTML markup on the page could also be factors in this issue. The fix for this issue wasn't in the component code, as the onElementUpdate callback was simply doing what it was meant to do: clean up. Moreover, other components could have produced the same issue. So, as the customer suggested, the fix consists in adding conditionals to notifyAllOnElementUpdateCallbacks() (in the bridge code) to ensure that the element being processed still exists. We were already checking for the existence of the callback function before invoking it, so this is just another standard check before that.
        Hide
        Arturo Zambrano added a comment -

        Testing notes: since there's no known way to reproduce the issue, just run your routine regression tests to see if this fix causes any regression. It is a fix in the bridge code, so it could potentially affect any component. However, since the fix itself consists in adding some defensive measures, it's very unlikely that this could cause any regressions.

        Show
        Arturo Zambrano added a comment - Testing notes: since there's no known way to reproduce the issue, just run your routine regression tests to see if this fix causes any regression. It is a fix in the bridge code, so it could potentially affect any component. However, since the fix itself consists in adding some defensive measures, it's very unlikely that this could cause any regressions.

          People

          • Assignee:
            Arturo Zambrano
            Reporter:
            Arran Mccullough
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: