ICEfaces
  1. ICEfaces
  2. ICE-9303

ICEfaces JS extends native prototypes overwriting expected behaviours

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: 4.0.BETA, 4.0
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      *
    • Assignee Priority:
      P1

      Description

      As a compatibility and convenience measure, ICEfaces extends the prototypes of native JS types with a variety of helper functions and shims for advanced ECMAscript features.

      However the extend ends up overwriting functions that have existing native implementations. This is generally discouraged for a few of reasons, primarily that this has a global effect, and that the overwriting implementation may not provide the expected functionality to 3rd party scripts, or that the overwriting implementation is slower due to being evaluated JS versus native C JSVM code.

      I encountered this issue when attempting to use Array.prototype.filter to filter a NodeList as recommended by MDN documentation and it broke due to our implementations assumptions. As higher-order JS techniques like this become more popular we run greater risk of being incompatible.

      A naive change, if possible would be to make extend not overwrite. If extend is used with the expectation of overwriting throughout or code this may not be possible however. In that case we may need to alter the calls to extend prototypes to indicate explicitly not to overwrite.

        Issue Links

          Activity

          Nils Lundquist created issue -
          Nils Lundquist made changes -
          Field Original Value New Value
          Assignee Nils Lundquist [ nils.lundquist ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #35644 Wed May 29 14:43:01 MDT 2013 nils.lundquist Revert: ICE-9109 - Prevented prototype extension from overwriting existing native implementations. Pushed to 3.4. Future effort captured in ICE-9303.
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/compat/core/src/main/javascript/prototype/prototype.js
          Hide
          Arturo Zambrano added a comment -

          I went on with modifying all occurrences of calls to Object.extend() in all compat scripts, adding a flag at the end to overwrite the properties in the destination object, and the regressions reported by QA seem to be solved. At least, when testing the component-showcase app, everything seems to work fine, including the issues with D&D and ice:selectInputText. Actually, only four files in the compat project make use of Object.extend(); they are extras.js, dragdrop,js, effect.js, and prototype.js. They are quite big, though. It seems like throughout the codebase, including the prototype library itself, the expected behaviour of Object.extend() is to overwrite properties with the same name. The online documentation hints at that idea:
          http://prototypejs.org/doc/latest/language/Object/extend/index.html

          This is not committed yet, but I'll keep it in a separate copy of the repository on my machine. I'll commit it after the 3.3-EE code freeze for further testing, unless otherwise instructed.

          Show
          Arturo Zambrano added a comment - I went on with modifying all occurrences of calls to Object.extend() in all compat scripts, adding a flag at the end to overwrite the properties in the destination object, and the regressions reported by QA seem to be solved. At least, when testing the component-showcase app, everything seems to work fine, including the issues with D&D and ice:selectInputText. Actually, only four files in the compat project make use of Object.extend(); they are extras.js, dragdrop,js, effect.js, and prototype.js. They are quite big, though. It seems like throughout the codebase, including the prototype library itself, the expected behaviour of Object.extend() is to overwrite properties with the same name. The online documentation hints at that idea: http://prototypejs.org/doc/latest/language/Object/extend/index.html This is not committed yet, but I'll keep it in a separate copy of the repository on my machine. I'll commit it after the 3.3-EE code freeze for further testing, unless otherwise instructed.
          Arturo Zambrano made changes -
          Link This issue blocks ICE-9259 [ ICE-9259 ]
          Arturo Zambrano made changes -
          Link This issue depends on ICE-9244 [ ICE-9244 ]
          Arturo Zambrano made changes -
          Link This issue depends on ICE-9244 [ ICE-9244 ]
          Arturo Zambrano made changes -
          Link This issue blocks ICE-9244 [ ICE-9244 ]
          Nils Lundquist made changes -
          Assignee Priority P1 [ 10010 ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #36971 Tue Jul 16 12:14:11 MDT 2013 nils.lundquist ICE-9303 - Prevent Prototype from overwriting native implementations.
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/compat/core/src/main/javascript/prototype/prototype.js
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #36972 Tue Jul 16 12:16:28 MDT 2013 nils.lundquist Revert: MOBI-832 - convert all uses of Array.each, Array.filter and Array.map to for loops to avoid prototype incompatibility
           
           Incompatibility with Prototype resolved with resolution of ICE-9303.
          Files Changed
          Commit graph MODIFY /icemobile/trunk/icemobile/jsf/components/component/resources/org.icefaces.component.util/component.js
          Hide
          Nils Lundquist added a comment -

          Revision #36971
          Committed by nils.lundquist
          5 minutes ago
          ICE-9303 - Prevent Prototype from overwriting native implementations.

          Show
          Nils Lundquist added a comment - Revision #36971 Committed by nils.lundquist 5 minutes ago ICE-9303 - Prevent Prototype from overwriting native implementations.
          Nils Lundquist made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #37009 Wed Jul 17 17:44:32 MDT 2013 nils.lundquist ICE-9303 - Hack for Prototype Enumerable to avoid deep copy infinite loop.
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/compat/core/src/main/javascript/prototype/prototype.js
          Ken Fyten made changes -
          Fix Version/s 4.0 [ 11382 ]
          Ken Fyten made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved: