#317 bug
fearphage

Prototype overwrites native array functionality

Reported by fearphage | August 31st, 2008 @ 10:41 PM | in 1.6.1

Prototype is overwriting the native implementations of the array extensions of js version 1.6: http://developer.mozilla.org/en/New_in_JavaScript_1.6#Array_extras

Array#every Array#filter Array#map Array#some

The native implementations would be faster.

Comments and changes to this ticket

  • John-David Dalton
  • fearphage

    fearphage September 11th, 2008 @ 01:46 PM

    To illustrate how unfortunate this is, I am currently working on a widget-like project and the owner of the site has included prototype. The native code I would like to use is:

    @@@ javascript: Array.prototype.filter.call(document.getElementsByTagName('input'), function(elem) { return ((elem.type == 'checkbox') && (elem.className == 'catpants') && elem.checked); });

    
    
    However, prototype overwrites Array#filter with its custom code and when I execute this it throws because NodeList#each is not a function. It would be nice if prototype at least saved a native reference for me to use. I don't want to use prototype for the widget because i would like it to be independent of js libraries.
    
  • John-David Dalton

    John-David Dalton September 11th, 2008 @ 03:23 PM

    Are you planning on supporting IE6/7, Opera 9.25 ? Prototype should check for the natives. But I think it should work identical to the natives. If Native Array#filter allows NodeLists and other Array-Like objects then Prototype's Array#filter should allow the same thing.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 03:40 PM

    • → Assigned user changed from “” to “Juriy Zaytsev”
    • → Tag changed from “array native needs_patch performance” to “array native needs_patch needs_tests performance”
    • → Milestone changed from “” to “1.6.1”
    • → State changed from “new” to “enhancement”

    Just steal it from an iframe : )

    
    document.body.innerHTML += '<iframe id="__iframe" style="display:none"></iframe>';
    var filter = $('__iframe').contentWindow.Array.prototype.filter;
    filter.call([1,2,3,4,5], function(n){ return n>3 }); // [4,5]
    

    I'll see what we can do about this.

  • fearphage

    fearphage September 11th, 2008 @ 03:58 PM

    • → Assigned user cleared.
    • → Tag changed from “array native needs_patch needs_tests performance” to “array native needs_patch performance”

    Indeed i am supporting them and I don't want to overwrite the customer's code so I'm working around it.

    
    if (!Array.prototype.filter) {
      Array.prototype.filter = function(fn, that) {
        var result = [];
        for (var i = 0, length = this.length; i < length; i++) {
          if (i in this) {
            var value = this[i]; // in case fn mutates this
            if (fn.call(that, value, i, this))
              result.push(value);
          }
        }
        return result;
      };
    }
    

    arguments is a good example of something that is not an array that can be passed to all the array functions though.

    A quick way to convert things into an array: Array.prototype.slice.call(arguments)

    I also use it to copy things into arrays.

    Could this replace $A?

  • fearphage

    fearphage September 11th, 2008 @ 04:00 PM

    • → Assigned user changed from “” to “Juriy Zaytsev”
    • → Tag changed from “array native needs_patch performance” to “array native needs_patch needs_tests performance”

    Weird system. I posted after you and cleared stuff. Sorry.

    Also, it is not an enhancement. It is a bug. You are preventing native functionality from taking place.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 04:03 PM

    • → Assigned user cleared.
    • → Tag changed from “array native needs_patch needs_tests performance” to “array native needs_patch performance”

    Could this replace $A?

    Sort of. $A also delegates its logic to an object if an object implements toArray.

  • John-David Dalton

    John-David Dalton September 11th, 2008 @ 04:18 PM

    • → State changed from “enhancement” to “bug”

    @fearphage Once Prototype is wrapped in a closure I think there will definetly be something like:

    
    var slice = Array.prototype.slice;
    //....
    slice.call(arguments, 0); can be used.
    

    AFAIK IE errors on slice.call(nodeList, 0) so it cannot be used for everything. There are ways to detect node lists and we could add an Object.isNodeList() method that would aid in detection and use $A() for the nodeLists or perform a one time capability test and use slice.call if supported and $A if not.

  • fearphage

    fearphage September 11th, 2008 @ 04:22 PM

    
    (function(slice) {
     $A = function() {
        if (typeof this.toArray == 'function')
          return this.toArray();
        return slice.apply(this, arguments);
      };
    })(Array.prototype.slice)
    

    Just throwing that out there. Slice also does what clone does.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 04:39 PM

    @fearphage

    I don't see a reason to change $A to use slice. Current approach does its job just fine. Besides, nothing is stopping developer from using slice where appropriate.

    Am I missing something?

  • fearphage

    fearphage September 11th, 2008 @ 05:36 PM

    Native is almost guaranteed faster than anything else. That's the only reason I would suggest.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 06:05 PM

    @fearphage

    Native array methods - yes, but not slice vs $A.

    Array.prototype.slice.call(arrayLikeObj) has almost identical performance as $A(arrayLikeObj). For short-length objects as function arguments, the difference is barely noticeable. For longer collections (e.g. NodeList's) it's almost negligible.

  • fearphage

    fearphage September 11th, 2008 @ 06:35 PM

    For short-length objects as function arguments, the difference is barely noticeable. For longer collections (e.g. NodeList's) it's almost negligible.

    Please try this 1,000 times with a 1,000 element node list and tell me again that its negligible. It is not. I am from the belief system that a performance increase is a performance increase is a performance increase. To translate: if you trim 5ms off of 10 calls, you earned 50ms. That's 50ms faster than before.

    Look at your performance numbers here: http://prototype.lighthouseapp.c...

    // old: 57ms // new: 30ms

    That's 27ms which is negligible but still important. I don't think only a few ms faster is something to ignore.

  • Juriy Zaytsev

    Juriy Zaytsev September 11th, 2008 @ 07:17 PM

    @fearphage

    
    // FF 3.0.1, Mac OS X 10.5
    
    var arr = document.getElementsByClassName('foo');
    // arr.length; // 1000
    
    console.time('$A');
    for (var i=0;i<100; i++) {
      $A(arr);
    }
    console.timeEnd('$A');
    
    console.time('slice');
    for (var i=0;i<100; i++) {
      Array.prototype.slice.call(arr);
    }
    console.timeEnd('slice');
    
    $A: 190ms
    slice: 125ms
    

    You're right.

    slice is ~50% faster when applied on a 1000-length collection 100 times. Whether this is negligible is a subjective argument. e.g. "function patch" that you cited has almost 100% improvement when run 1000 times (not 100,000 as in slice example).

    All the nitpicking aside, I agree that even such a small difference could be important. We actually have plans to replace $A with slice where appropriate (but leaving $A internals as they currently are).

    I would just like to focus this discussion on Array methods, not $A performance ; )

  • howardr

    howardr September 13th, 2008 @ 03:52 AM

    @Juriy Zaytsev

    I am assuming you will not be adding the iframe to the DOM via innerHTML:

    
    
    document.body.innerHTML += '<iframe id="__iframe" style="display:none"></iframe>';
    
    

    That will remove any event handlers bound to elements

  • John-David Dalton

    John-David Dalton September 14th, 2008 @ 07:32 PM

    
    (function() {
      var tmp = [], window = this,
       natives = ['Array', 'Number', 'String'];
    
      function extend(constructor, methods) {
        methods = methods.split(' ');
        for(var i = 0, m; m = methods[i++];)
          Prototype[constructor].prototype[m] = window[constructor].prototype[m];
      }
      function setConstructor(constructor, value) {
        value.constructor = Prototype[constructor];
        return value;
      }
      function setProto(constructor, value) {
        value.__proto__= Prototype[constructor].prototype;
        return setConstructor(constructor, value);
      }
    
      if ((tmp.__proto__ = { }) && !tmp.push) {
        // Firefox, Safari
        for (var i = 0, n; n = natives[i++];) {
          Prototype[n] = (n == 'Array') ?
            function(value) { return setProto(n, value) } :
            function(value) { return setProto(n, new window[n](value)) };
        }
        extend('Array', 'concat every filter indexOf join forEach lastIndexOf map pop push ' +
         'reduce reduceRight reverse shift slice some sort splice unshift');
    
        extend('String', 'charAt charCodeAt concat indexOf lastIndexOf match replace search ' +
         'slice split substr substring toLowerCase toUpperCase');
      }
      else {
        // IE 6/7, Opera
        var idoc, iroot, docElement = document.documentElement,
         iframe = document.createElement('iframe');
        iframe.style.cssText = 'position:absolute;display:none;' +
         'left:-5px;top:-5px;width:0px;height:0px;overflow:hidden';
    
        docElement.insertBefore(iframe, docElement.firstChild);
        (idoc = window.frames[0].document).open();
        idoc.write('<script>parent.Prototype._iroot = this;<\/script>');
        idoc.close();
        iframe.parentNode.removeChild(iframe);
    
        iroot = Prototype._iroot;
        delete Prototype._iroot;
    
        for (var i = 0, n; n = natives[i++];) {
          Prototype[n] = (n == 'Array') ?
            function(value) { return setConstructor(n, iroot.Array.prototype.slice.call(value, 0)) } :
            function(value) { return setConstructor(n, new iroot[n](value)) };
          Prototype[n].prototype = iroot[n].prototype;
        }
      }
    })();
    
    
    /* TESTS */
    
    var foo = Prototype.Array([]);
    foo[2] = 'apple';
    foo[1] = 'orangle';
    foo[0] = 'banana';
    alert(foo.length); //3
    
    Prototype.Array.prototype.customized = function(){ return this };
    alert(typeof foo.customized); // function
    
    var bar = ['a', 'b', 'c'];
    alert(typeof bar.customized); // undefined
    
    bar = Prototype.Array.prototype.slice.call(bar, 0);
    alert(typeof bar.customized); // function
    
  • fearphage

    fearphage September 13th, 2008 @ 03:46 PM

    The only problem with that is that arrays created with the clean iframe are no longer instances of the array created with the main window.

    
    bar = Prototype.Array.prototype.slice.call(bar, 0);
    alert(typeof bar.customized); // function
    alert(bar instanceof Array); // false
    

    Think of JSON.stringify for instance which uses instanceof. Now you have to check for an instance of an array from both windows. So anything I use this "clean array" on will not be stringify-able.

  • John-David Dalton

    John-David Dalton September 13th, 2008 @ 04:56 PM

    If you are using Prototype you can use Object.isArray() which will still return true for Arrays from iframes and you can use Prototype's JSON methods.

    Ideally we would be only extending the "clean" array so that the native one remains untouched.

    
    Object.extend(Prototype.Array.prototype, Enumerable);
    Object.extend(Prototype.Array.prototype, ....);
    

    We would make $A() return a new Prototoype.Array or people could alias it as var array = Prototype.Array or we could have a Object.toArray() which does the Prototype.Array.prototype.slice.call(arguments[0], 0);

    Either way you end up with an array that has the toJSON method attached and it becomes a non-issue.

  • fearphage

    fearphage September 13th, 2008 @ 06:31 PM

    I think you are missing the point. I also believe you are underestimating the importance of this bug. I am not trying to make my project dependent on any library.

    I want to use native functionality that is built into the browser but Prototype is preventing this from taking place.

    I want my widget to be library agnostic because I don't want there to be any external dependencies.

  • John-David Dalton

    John-David Dalton September 13th, 2008 @ 06:34 PM

    I was proposing Prototype use the "clean" Array so it left the normal native array untouched. I think that is addressing the problem head on. It would allow any other library to interact with normal arrays without incident.

  • fearphage

    fearphage September 13th, 2008 @ 08:04 PM

    @John-David Dalton: that sounds like a marvelous idea. I wasn't viewing your solution properly. That seems like a global solution to leave all native properties, objects, and methods untouched.

  • Juriy Zaytsev

    Juriy Zaytsev September 13th, 2008 @ 08:09 PM

    @fearphage

    The importance of the issue is actually pretty clear. The naive iframe workaround (that I showed before) was obviously a temporary "solution" that one could use. This workaround is not ideal, of course.

    The reason why we can't just go ahead and stop replacing native methods is back compatibility. Prototype.js has made some unfortunate decisions that we now have to keep for compatibility reasons. Replacing native methods is one of them (extending host objects/host objects' [[Prototype]]'s is another, etc.)

  • fearphage

    fearphage September 13th, 2008 @ 08:29 PM

    Can't you deprecate them and just say Prototype is getting better? This is one of the first times where Prototype has unrecoverably contaminated the native environment for me.

  • Juriy Zaytsev

    Juriy Zaytsev September 13th, 2008 @ 11:32 PM

    • → Tag cleared.

    Yep, we need to deprecate prototype's map, filter, every, some and reduce. Next release - 1.6.0.3 - is a bugfix/performance optimization only, so the deprecation will need to wait till 1.6.1 : /

  • fearphage

    fearphage September 14th, 2008 @ 11:24 AM

    Thinking about this, the only new problem i see will be that you will be forcing users to use Prototype more.

    Let's say I do something and returns an instance of Prototype.Array. if (pArray instanceof Array) will now fail. I am forced to use Object.isArray(pArray). I'm not sure if this is a huge problem but it is something to consider nonetheless.

    Side note: The posting of unpreviewable, uneditable comments sucks.

  • fearphage

    fearphage September 14th, 2008 @ 11:32 AM

    Have you tested this solution in IE6? IE6 is complaining about this.each not being a function which is what I got before I started using the work around.

  • fearphage

    fearphage September 14th, 2008 @ 12:30 PM

    Scratch that last one. My problem was document.domain related. If you set document.domain before creating the iframe, the iframe has to set document.domain as well. Maybe that is something for the release notes.

    "Include prototype before setting document.domain"

  • John-David Dalton

    John-David Dalton September 14th, 2008 @ 05:54 PM

    I tried setting the document.domain to match the pages via in .write() call and I get a permission denied error. Ahh do to an IE6/7 bug you can only set its document.domain after the iframe loads so calling. idoc.domain = document.domain; should work...

    Updated

    Didn't error but didn't work :(

  • fearphage

    fearphage September 14th, 2008 @ 05:51 PM

    It's not possible unless you load a special page in the iframe which then makes the whole process asynchronous which sucks. You have to capture your clean instances before document.domain is set.

  • John-David Dalton

    John-David Dalton September 14th, 2008 @ 06:33 PM

    There is also a memory leak associated with cross-frame communication, but the way we handle it avoids the leak. http://novemberborn.net/javascript/edgvl

  • John-David Dalton

    John-David Dalton September 14th, 2008 @ 07:15 PM

    Updated my example to use an alternate method in Safari and Firefox that manipulates the object instances proto property.

    However, using this approach makes

    
    var foo = Prototype.Array([]);
    alert(foo instanceof Array); -> true for Firefox,Safari and  false for IE, Opera
    

    This shouldn't be a big deal. Discussion about namespacing Prototype. http://groups.google.com/group/prototype-core/browse_thread/thread/16d0517ecc605a00

Please Login or create a free account to add a new comment.

You can update this ticket by sending an email to from your email client. (help)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

The Prototype JavaScript library.

Shared Ticket Bins