Giter VIP home page Giter VIP logo

Comments (18)

kriskowal avatar kriskowal commented on July 19, 2024

Thanks, looking for a volunteer to spec and patch on Windows. @domenic @bryanforbes @michaelficarra?

from es5-shim.

michaelficarra avatar michaelficarra commented on July 19, 2024

Sorry, no Windows for me.

from es5-shim.

termi avatar termi commented on July 19, 2024

I wrote a path to fix this issue (need tests)
[update: INCORRECT]

var array_splice = Array.prototype.splice
Array.prototype.splice = function(start, deleteCount) {
    var originalLength = this.length
        , result = array_splice.apply(this, arguments)
        , newLength = originalLength - deleteCount + arguments.length - 2
    ;

    if(arguments.length > 2 && newLength != this.length && this["__prevSpliceLength__"] !== void 0) {
        array_splice.call(this, this["__prevSpliceLength__"], start);
    }

    this["__prevSpliceLength__"] = originalLength;

    return result
}

The only BAD thing is obligatorily to save previous array.length in "prevSpliceLength" variable.
In my experiments, this error does not occur then Array.prototype.splice call with second parameter > 0, so I don't consider this parameter in the algorithm.

If you find this algorithm satisfactory, I can create a pull request with bunch of tests

from es5-shim.

nightwing avatar nightwing commented on July 19, 2024

@termi nice and simple, but doesn't cover all the cases :(

var makeArray = function(l) {
    var a = [];
    while (l--) {
        a.unshift(Array(l + 1).join(" ") + l)
    }
    return a
}
var log = function(a){console.log(a.join("\n"))}
var b1 = makeArray(6);

b1.splice(b1.length - 1, 1, "");
b1.splice(0, 1, "");
b1.splice(0, b1.length);
log(b1);

var a1 = makeArray(36);

var args = [0, 0];
args.push.apply(args, a1);
b1.splice.apply(b1, args);

b1.splice(3, 0, "");
log(b1);

from es5-shim.

kriskowal avatar kriskowal commented on July 19, 2024

@nightwing do you have a moment to add tests to the suite? There are plenty of examples to pattern after.

from es5-shim.

termi avatar termi commented on July 19, 2024

In my experiments I found two different bug with [].splice in IE8:
{ Let the Δ would be the start index of garbage data in array after [].splice }

  1. Δ = array.length in previous [].splice.
    This bug occurs after [].splice
  2. Δ is 8, 16, 32, 64 ... 2^n, where 2^n is a nearest value to [].splice first argument.
    This bug occurs after [].unshift

Continues testing

from es5-shim.

 avatar commented on July 19, 2024

Actually, there's an easier way to detect JScript's incomplete Array#splice implementation—omitting the deleteCount argument will not remove any elements from the array (the correct behavior is to remove every element from that index onward).

if (![1].splice(0).length) {
  // ...
}

Here's a line-by-line implementation of the Array::splice method as described in the spec. Time permitting, I can go ahead and submit a proper pull request with tests later this weekend.

from es5-shim.

termi avatar termi commented on July 19, 2024

if (![1].splice(0).length)
This detect gives wrong true in IE6/7 that doesn't have "data duplication" bug in [].splice

from es5-shim.

 avatar commented on July 19, 2024

Right, but it's still a broken native implementation that needs to be patched. Instead of testing for incorrect 'deleteCount' handling (IE 6-8) and data duplication (IE 8) separately, why not use one test and patch both?

Edit: Ah, but it would still be good to have some test cases for the unit tests. Never mind.

from es5-shim.

termi avatar termi commented on July 19, 2024

@nightwing, @kitcambridge
I wonder if we can replace Array.prototype.unshift to [].splice analog to reduce [].splice bugs count.
[update: INCORRECT]

var array_splice = Array.prototype.splice;
Array.prototype.unshift = function() {
    var args = Array.prototype.slice.call(arguments);
    array_splice.apply(args, [0, 0, 0, 0]);
    array_splice.apply(this, args);
    return this.length;
}
Array.prototype.splice = function(start, deleteCount) {
    var originalLength = this.length
        , result = array_splice.apply(this, arguments)
        , newLength = originalLength - deleteCount + arguments.length - 2
    ;

    if(arguments.length > 2 && newLength != this.length && this["__prevSpliceLength__"] !== void 0) {
        array_splice.call(this, this["__prevSpliceLength__"], start);
    }

    this["__prevSpliceLength__"] = originalLength;

    return result
}

But It can be more [].splice bugs with [].splice interaction with other Array functions

from es5-shim.

nightwing avatar nightwing commented on July 19, 2024

@termi this doesn't fix the snippet from my comment (unshift isn't important in it since the same bug happens with push)

@kitcambridge with your splice implementation, ace works well, but splice is 2-3 times slower than native (tested on relatively small arrays)

from es5-shim.

termi avatar termi commented on July 19, 2024

@nightwing Another attempt:
[update: INCORRECT]

function findNearestN_pow_X(value) {
    var i = 1;
    --value;

    while (value > 0) {
        i <<= 1;
        value >>= 1;
    }

    return i;
};

var _Array_splice_ = Array.prototype.splice;
Array.prototype.splice = function(start, deleteCount) {
    var originalLength = this.length
        , result = _Array_splice_.apply(this, arguments)
        , newLength = originalLength - deleteCount
        , indexOfTrash1 = this["__prevSpliceLength__"]
        , indexOfTrash2 = this["__firstSpliceLength__"]
        , i
    ;

    if(newLength < 0) {
        newLength = 0;
    }

    newLength += (arguments.length - 2);

    if(arguments.length > 2 && newLength != this.length && (indexOfTrash1 !== void 0 || indexOfTrash2 !== void 0)) {
        i = -1;
        if(indexOfTrash1 !== void 0) {
            while(this[++i] === this[i + 1 + start]) {
                //NULL
            }
        }

        _Array_splice_.call(this, i === start ? indexOfTrash1 : indexOfTrash2, start);
    }

    this["__prevSpliceLength__"] = originalLength;
    if(indexOfTrash2 === void 0) {
        this["__firstSpliceLength__"] = findNearestN_pow_X(originalLength);
    }

    return result
}

It's pass your comment. But I am not sure about this fix. Writing tests now

from es5-shim.

nightwing avatar nightwing commented on July 19, 2024

it passes the comment but still fails on ace :(
link in ext.js code suggests that it may be quite tricky to cover all possible cases

from es5-shim.

termi avatar termi commented on July 19, 2024

@nightwing I think, I found perfect solution:
[update: FIXEDx2]

var _Array_splice_ = Array.prototype.splice
     , _Array_slice_ = Array.prototype.slice
;
Array.prototype.splice = function(start, deleteCount) {
    var result
        , args = _Array_slice_.call(arguments, 2)
        , addElementsCount = args.length
    ;

    if(!arguments.length) {
        return [];
    }

    if(start === void 0) { // default
        start = 0;
    }
    if(deleteCount === void 0) { // default
        deleteCount = this.length - start;
    }

    if(addElementsCount > 0) {
        if(deleteCount <= 0) {
            if(start == this.length) { // tiny optimisation #1
                this.push.apply(this, args);
                return [];
            }

            if(start == 0) { // tiny optimisation #2
                this.unshift.apply(this, args);
                return [];
            }
        }

        // Array.prototype.splice implementation
        result = _Array_slice_.call(this, start, start + deleteCount);// delete part
        args.push.apply(args, _Array_slice_.call(this, start + deleteCount, this.length));// right part
        args.unshift.apply(args, _Array_slice_.call(this, 0, start));// left part

        // delete all items from this array and replace it to 'left part' + _Array_slice_.call(arguments, 2) + 'right part'
        args.unshift(0, this.length);

        _Array_splice_.apply(this, args);

        return result;
    }

    return _Array_splice_.call(this, start, deleteCount);
}

I force deleteCount in [].splice to be always more then 0.

I replace splice to "push + unshift + Array_slice.call(arguments, 2)" implementation

from es5-shim.

nightwing avatar nightwing commented on July 19, 2024

this doesn't behave like splice,
see https://c9.io/nightwing/ace/workspace/build/ie8_test.html
i've tried modifying it to

Array.prototype.splice1 = function(start, deleteCount) {
    var bugfix, result, args = arguments;

    if(deleteCount <= 0 && args.length > 2) {
        args = _Array_slice_.call(args, 2);
        args.unshift(start, 1);
        if (start < this.length) {
            bugfix = true;
            args.push(this[start])
        }
    }
    result = _Array_Splice_.apply(this, args);

    return bugfix ? [] : result;
}

but this one doesn't work either.

probably there is no way to fix this without using full reimplementation.

from es5-shim.

termi avatar termi commented on July 19, 2024

@nightwing This is just my bug, I fixed it.

Thx for the url - it's more easy to test polyfill in this page then in http://ace.ajax.org/build/kitchen-sink.html

from es5-shim.

termi avatar termi commented on July 19, 2024

@kriskowal, @nightwing, @kitcambridge As soon as my fix in master branch, can you close this issue?

from es5-shim.

kriskowal avatar kriskowal commented on July 19, 2024

@termi, I presume you mean #143. It has been merged.

Please, anyone, feel free to reopen if there are further issues not addressed.

from es5-shim.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.