Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ie8: non-iterable properties valueOf, constructor. Patch Object.forEach #2613

Closed
qbolec opened this Issue Jun 13, 2014 · 8 comments

Comments

Projects
None yet
5 participants

qbolec commented Jun 13, 2014

In IE8 the constructor field if present and is a function is masked when iterating with for..in :

var x={a:7,constructor:function(){return 9;}}
'constructor' in x
true
for(var key in x)alert(key);
//only "a" pops out

This causes a bug in Object.each, in Object.merge, in Options.setOptions and so on.
This is particularly problematic for Neuro.js which insists on calling one of the options passed to initialize collection "constructor", as in this case constructor is usually a function (to be specific: a mootools Class extending Neuro.Model) which results in Neuro.js not working at all under ie8.

qbolec commented Jun 13, 2014

I've already filed GCheung55/Neuro#34

Contributor

GCheung55 commented Jun 27, 2014

Owner

ibolmo commented Jul 2, 2014

Well Class.Extras.js#L112 skips constructor because it's lacking the 'on' prefix in the name.

Owner

ibolmo commented Jul 2, 2014

And constructor in IE is reserved/special field in an object). Nothing we can do about that.

@ibolmo ibolmo closed this Jul 2, 2014

qbolec commented Jul 2, 2014

I am surprised that you've attempted this problem from the angle of setOptions, while the problem is much easier solved by patching Object.each function.
I would suggest to try to change lines around https://github.com/mootools/mootools-core/blob/master/Source/Core/Core.js#L308 to something like

Object.extend('forEach', function(object, fn, bind){
        var remember_about_constructor = 'constructor' in object;
        for (var key in object){
                if(key === 'constructor') remember_about_constructor = false;
                if (hasOwnProperty.call(object, key)) fn.call(bind, object[key], key, object);
        }
        if(remember_about_constructor){
                key = 'constructor';
                if (hasOwnProperty.call(object, key)) fn.call(bind, object[key], key, object);
        }
});

or something similar.

Owner

ibolmo commented Jul 2, 2014

I'm still unsure why you'd like to use constructor. Why not use init or start?

Owner

kamicane commented Jul 2, 2014

If MooTools is to support browsers that are < ie9, it should definitely provide a patched Object.forEach / each. Reason: constructor, toString, valueOf, and other properties set as non-iterable.

Examples:

https://github.com/mout/mout/blob/v0.1.0/src/object/forIn.js#L6
https://github.com/lodash/lodash/blob/2.4.1/dist/lodash.compat.js#L85
https://github.com/kamicane/prime/blob/master/index.js#L21

@kamicane kamicane reopened this Jul 2, 2014

@ibolmo ibolmo modified the milestones: 1.5.1, 1.5.2 Jul 3, 2014

@ibolmo ibolmo changed the title from Object.each, setOptions etc. fail to iterate over 'constructor' field in ie8 to ie8: non-iterable properties valueOf, constructor. Patch Object.forEach Nov 11, 2014

@ibolmo ibolmo added bug IE labels Nov 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment