Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSIE8 inserts empty nodes #31

Closed
IvanSanchez opened this issue Apr 1, 2015 · 4 comments
Closed

MSIE8 inserts empty nodes #31

IvanSanchez opened this issue Apr 1, 2015 · 4 comments

Comments

@IvanSanchez
Copy link
Contributor

I've found that, after adding enough data, MSIE8 creates nodes with infinite/-infinite coordinates (this will make the bush crash when more items are added, or when searching for an item).

The simplest reproducible case I could come up with is this:

var bush = rbush();

// Force the bush to split much earlier
bush._maxEntries = 2;
bush._minEntries = 1;

bush.insert( [-449,235,-329,277,] );
bush.insert( [-449,262,-425,286,] );
bush.insert( [-215,200,-49,242,]  );
bush.insert( [-215,227,-191,251,] );
bush.insert( [222,98,298,140,]    );
bush.insert( [222,125,246,149,]   );

console.log(bush.data.children[1].bbox);

// On MSIE8:          [Infinity, Infinity, -Infinity, -Infinity]
// Any other browser: [ -449, 200, -49, 286 ]

If _minEntries and _maxEntries are set to the default values of 4 and 9, the bug is reproducible after inserting about 30-40 items.

The bug is reproducible with MSIE8, or MSIE11 emulating MSIE8. I haven't tried polyfilling ecmascript5 functionality.

@mourner
Copy link
Owner

mourner commented Apr 6, 2015

WAT!!! That's seriously WTF. No idea how that happens. Really curious to find the cause...

@elemoine
Copy link

elemoine commented Apr 6, 2015

@IvanSanchez no JS error? (IE 8 does not have Array.prototype.indexOf for example.)

@mourner
Copy link
Owner

mourner commented Apr 6, 2015

Doesn't look like there's any error thrown. Also, indexOf is only used in the removal routine. Should be something on insert/split.

@IvanSanchez
Copy link
Contributor Author

I found something off while tracing around here: https://github.com/mourner/rbush/blob/master/rbush.js#L317 , and then I found http://www.to-string.com/2012/05/29/fixing-splice-in-older-versions-of-internet-explorer-8-and-olders/

Also note that there is a ecmascript shim for Array.prototype.splice(): https://github.com/es-shims/es5-shim/blob/master/es5-shim.js#L365

At this moment I'm partial to not "fixing" this and just making a note in the READMEs.

IvanSanchez added a commit to MazeMap/rbush that referenced this issue May 20, 2015
mourner added a commit that referenced this issue May 20, 2015
Note about IE8 in readme in order to forget about #31.
simon04 referenced this issue in MazeMap/Leaflet.LayerGroup.Collision Dec 12, 2015
simon04 added a commit to simon04/Leaflet.LayerGroup.Collision that referenced this issue Dec 13, 2015
mourner/rbush#31 is fixed since 1.4.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants