ICEfaces
  1. ICEfaces
  2. ICE-7325

ACE components using direct references to $

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1-Beta
    • Fix Version/s: 2.1-Beta2, 3.0
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      -
    • Assignee Priority:
      P1
    • Affects:
      Compatibility/Configuration
    • Workaround Exists:
      Yes
    • Workaround Description:
      Hide
      <script type="text/javascript">
      $ = jQuery;
      </script>
      Show
      <script type="text/javascript"> $ = jQuery; </script>

      Description

      A user is receiving the following JS error when using the ACE components, specifically the ace:dataTable:

      $(this.jqId + " th.ui-sortable-column .ui-sortable-control") is null.

      This can be reproduce in the ACE component suite showcase found in 2.1 Beta. He has found that the $ object is not referencing the jQuery object and that the solution is to add "$ = jQuery" to the page like so:

      <script type="text/javascript">
      $ = jQuery;
      </script>

      However he fears that this may break more complex pages. JQuery should ideally be used in no-conflict mode and the component JS should not be directly accessing the global $ object but instead should I either use jQuery" or re-assign to a member variable.
      1. simpleDatatableTest_output.html
        14 kB
        Pierre Asselin
      2. simpleDatatableTest.xhtml
        3 kB
        Tyler Johnson
      1. screenshot-01.png
        197 kB
      2. screenshot-02.png
        163 kB

        Activity

        Hide
        Tyler Johnson added a comment -

        Attaching customer's sample file showing their implementation.

        Show
        Tyler Johnson added a comment - Attaching customer's sample file showing their implementation.
        Hide
        Pierre Asselin added a comment -

        The workaround mentioned was just to make it work, it is not a viable solution. Also, the salesforce case includes details on changes I've made to the ICEfaces code to fix the issue on for many components that require a separate js file.

        I've just noticed that the ace:tableConfigPanel outputs:
        onclick="$(ice.ace.escapeClientId('master:table_tableconf')).toggle()"
        Which also throws a javascript error because it is accessing $ directly. I haven't tried all components but it looks like all components should be reviewed to correct such cases.

        Show
        Pierre Asselin added a comment - The workaround mentioned was just to make it work, it is not a viable solution. Also, the salesforce case includes details on changes I've made to the ICEfaces code to fix the issue on for many components that require a separate js file. I've just noticed that the ace:tableConfigPanel outputs: onclick="$(ice.ace.escapeClientId('master:table_tableconf')).toggle()" Which also throws a javascript error because it is accessing $ directly. I haven't tried all components but it looks like all components should be reviewed to correct such cases.
        Hide
        Ken Fyten added a comment -

        List of reported affected files:

        ace/component/resources/icefaces.ace/core/core.js **
        ace/component/resources/icefaces.ace/datatable/datatable.js **
        ace/component/resources/icefaces.ace/dnd/dragdrop.js
        ace/component/resources/icefaces.ace/forms/forms.js *
        ace/component/resources/icefaces.ace/panel/panel.js
        ace/component/resources/icefaces.ace/progressbar/progressbar.js
        ace/component/resources/icefaces.ace/resizable/resizable.js
        ace/component/resources/icefaces.ace/tableconf/tableconf.js
        ace/component/resources/icefaces.ace/util/combined.js
        ace/component/resources/icefaces.ace/util/combined.js.tmp.js
        ace/component/resources/icefaces.ace/calendar/calendar.js *

        Show
        Ken Fyten added a comment - List of reported affected files: ace/component/resources/icefaces.ace/core/core.js ** ace/component/resources/icefaces.ace/datatable/datatable.js ** ace/component/resources/icefaces.ace/dnd/dragdrop.js ace/component/resources/icefaces.ace/forms/forms.js * ace/component/resources/icefaces.ace/panel/panel.js ace/component/resources/icefaces.ace/progressbar/progressbar.js ace/component/resources/icefaces.ace/resizable/resizable.js ace/component/resources/icefaces.ace/tableconf/tableconf.js ace/component/resources/icefaces.ace/util/combined.js ace/component/resources/icefaces.ace/util/combined.js.tmp.js ace/component/resources/icefaces.ace/calendar/calendar.js *
        Hide
        Ted Goddard added a comment -

        Local declaration of the jQuery object should use jq rather than "$" due to potential confusion or conflict with other JavaScript libraries:

        (function(jq)

        {......}

        )(ice.ace.jq);

        The above onclick handler would be modified as follows:

        onclick="ice.ace.jq(ice.ace.escapeClientId('master:table_tableconf')).toggle()"

        Show
        Ted Goddard added a comment - Local declaration of the jQuery object should use jq rather than "$" due to potential confusion or conflict with other JavaScript libraries: (function(jq) {......} )(ice.ace.jq); The above onclick handler would be modified as follows: onclick="ice.ace.jq(ice.ace.escapeClientId('master:table_tableconf')).toggle()"
        Hide
        yip.ng added a comment - - edited

        See the first call-out in screenshot-01.png for a brief description of the technique.

        For a well-written explanation of the immediate function pattern, see chapter 4 in the book "JavaScript Patterns" by Stoyan Stefanov, 2010.

        See also http://docs.jquery.com/Using_jQuery_with_Other_Libraries.

        Show
        yip.ng added a comment - - edited See the first call-out in screenshot-01.png for a brief description of the technique. For a well-written explanation of the immediate function pattern, see chapter 4 in the book "JavaScript Patterns" by Stoyan Stefanov, 2010. See also http://docs.jquery.com/Using_jQuery_with_Other_Libraries .
        Hide
        yip.ng added a comment - - edited

        From Ted (and core team):

        The $ usages indicated by the second call-out in screenshot-01.png are fine and none of the component JS files need to be changed.

        Show
        yip.ng added a comment - - edited From Ted (and core team): The $ usages indicated by the second call-out in screenshot-01.png are fine and none of the component JS files need to be changed.
        Hide
        Pierre Asselin added a comment -

        Incorrect, all the files listed by Ken need to be changed. Not only that, but I noticed that some components like the datatable config panel write onclick handlers that use the global $. It looks like the primefaces code uses closures for $ but not the code that was added for the ace components. The $ signs in the second callout are outside of the closure pointed at by the first callout: the global $ is therefore used. All the files listed above do show the same problem and I have corrected them locally which makes me now able to use the 2.1 components. We do need this corrected if we are to use 2.1.

        Show
        Pierre Asselin added a comment - Incorrect, all the files listed by Ken need to be changed. Not only that, but I noticed that some components like the datatable config panel write onclick handlers that use the global $. It looks like the primefaces code uses closures for $ but not the code that was added for the ace components. The $ signs in the second callout are outside of the closure pointed at by the first callout: the global $ is therefore used. All the files listed above do show the same problem and I have corrected them locally which makes me now able to use the 2.1 components. We do need this corrected if we are to use 2.1.
        Hide
        Pierre Asselin added a comment -

        To help you understand the issue, I'm attaching the html that gets generated by the test page. It shows that all scripts are scripts added by ICEfaces and that the prototype.js library gets added after the jquery one. So when you use ace components alongside ice components, it looks like you'll get that issue (I think the prototype autocompleter is used by ICEfaces). It just goes to show it's never a good idea to rely on a global scope variable outside of your namespace in framework code such as ICEfaces, particularly if the variable is $ which is often overridden. It is quite easy for the framework to keep a reference to jQuery that it can trust and that will not be overridden like it is often the case with global $. I'm not talking about using $ inside a closure, that is a good way of doing so. Making sure your framework uses a reference it can trust will also make ICEfaces more robust and will make it easier for developers (who often don't know about these aspects of javascript programming). It also becomes harder for the using application to control the overriding of $ in a portlet scenario. I see only good reasons for not using the global $ in framework code.

        Show
        Pierre Asselin added a comment - To help you understand the issue, I'm attaching the html that gets generated by the test page. It shows that all scripts are scripts added by ICEfaces and that the prototype.js library gets added after the jquery one. So when you use ace components alongside ice components, it looks like you'll get that issue (I think the prototype autocompleter is used by ICEfaces). It just goes to show it's never a good idea to rely on a global scope variable outside of your namespace in framework code such as ICEfaces, particularly if the variable is $ which is often overridden. It is quite easy for the framework to keep a reference to jQuery that it can trust and that will not be overridden like it is often the case with global $. I'm not talking about using $ inside a closure, that is a good way of doing so. Making sure your framework uses a reference it can trust will also make ICEfaces more robust and will make it easier for developers (who often don't know about these aspects of javascript programming). It also becomes harder for the using application to control the overriding of $ in a portlet scenario. I see only good reasons for not using the global $ in framework code.
        Hide
        yip.ng added a comment -

        As suggested by Ted, $ globally replaced with ice.ace.jq. Visually inspected afterwards to double check. ice.ace.jq created in core.js using jQuery.noConflict().

        Exceptions:

        • 3rd party plugins already using recommended techniques mentioned at http://docs.jquery.com/Using_jQuery_with_Other_Libraries
        • combined.js and combined.js.tmp.js (listed above by Ken) are generated files
        • in datatable.js and tableconf.js, $ used in other variable names as well, e.g. $this, $val

        After jQuery.noConflict(), global $ no longer points to jQuery, and will be undefined if not taken over by another lib. You will get a JS error as shown in screenshot-02.png if you try to use it.

        All demos in comp. suite tested. (In FF. Not thoroughly.)

        Revision: 25981


        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/calendar/calendar.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/core/core.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/dnd/dragdrop.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/forms/forms.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/panel/panel.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/progressbar/progressbar.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/resizable/resizable.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/tableconf/tableconf.js
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/datatable/DataTableRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/datetimeentry/DateTimeEntry.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/datetimeentry/DateTimeEntryRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/dialog/DialogRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/dnd/DraggableRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/dnd/DroppableRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/progressbar/ProgressBarRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/resizable/ResizableRenderer.java
        Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/tableconfigpanel/TableConfigPanelRenderer.java

        Show
        yip.ng added a comment - As suggested by Ted, $ globally replaced with ice.ace.jq. Visually inspected afterwards to double check. ice.ace.jq created in core.js using jQuery.noConflict(). Exceptions: 3rd party plugins already using recommended techniques mentioned at http://docs.jquery.com/Using_jQuery_with_Other_Libraries combined.js and combined.js.tmp.js (listed above by Ken) are generated files in datatable.js and tableconf.js, $ used in other variable names as well, e.g. $this, $val After jQuery.noConflict(), global $ no longer points to jQuery, and will be undefined if not taken over by another lib. You will get a JS error as shown in screenshot-02.png if you try to use it. All demos in comp. suite tested. (In FF. Not thoroughly.) Revision: 25981 Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/calendar/calendar.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/core/core.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/dnd/dragdrop.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/forms/forms.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/panel/panel.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/progressbar/progressbar.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/resizable/resizable.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/resources/icefaces.ace/tableconf/tableconf.js Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/datatable/DataTableRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/datetimeentry/DateTimeEntry.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/datetimeentry/DateTimeEntryRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/dialog/DialogRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/dnd/DraggableRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/dnd/DroppableRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/progressbar/ProgressBarRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/resizable/ResizableRenderer.java Modified : /icefaces-ee/scratchpads/grimlock/icefaces/ace/component/src/org/icefaces/ace/component/tableconfigpanel/TableConfigPanelRenderer.java

          People

          • Assignee:
            yip.ng
            Reporter:
            Tyler Johnson
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: