Skip to content

Conversation

@Munter
Copy link
Contributor

@Munter Munter commented Feb 20, 2015

When running color-diff in an environment where Array.prototype has been amended with helper functions, the palette will some times return color objects with NaN in every dimension.

I tried using a for(var idx1; idx1 < a.length; idx1 += 1) loop, which would be cleaner, but when I do the tests are failing.

@markusn
Copy link
Owner

markusn commented Feb 20, 2015

Hi Munter! Thanks for contributing to color-diff!

I'd prefer the solution using the modified loop, it should work if you change the loop to:
for (var idx1 = 0; idx1 < a.length; idx1++) (the code you pasted missed the initialization of the index).

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.44%) to 97.81% when pulling 6aec030 on Munter:hasOwnProperty-fix into 8c74b04 on markusn:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.44%) to 97.81% when pulling 6aec030 on Munter:hasOwnProperty-fix into 8c74b04 on markusn:master.

@Munter Munter force-pushed the hasOwnProperty-fix branch from 6aec030 to e0a7c2e Compare February 20, 2015 15:02
@Munter
Copy link
Contributor Author

Munter commented Feb 20, 2015

Right, I guess I was a bit tired when I tried that version. I've switched to a traditional for-loop now and the tests are passing. I've also verified that this version doesn't experience the original problem I encountered

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.44%) to 97.81% when pulling e0a7c2e on Munter:hasOwnProperty-fix into 8c74b04 on markusn:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.44%) to 97.81% when pulling e0a7c2e on Munter:hasOwnProperty-fix into 8c74b04 on markusn:master.

@Munter Munter force-pushed the hasOwnProperty-fix branch from e0a7c2e to 467627e Compare February 20, 2015 15:04
@Munter
Copy link
Contributor Author

Munter commented Feb 20, 2015

And now with the hasOwnProperty check removed :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.25% when pulling 467627e on Munter:hasOwnProperty-fix into 8c74b04 on markusn:master.

markusn added a commit that referenced this pull request Feb 20, 2015
Guard against external libraries modifying Array.prototype
@markusn markusn merged commit 182233e into markusn:master Feb 20, 2015
@markusn
Copy link
Owner

markusn commented Feb 20, 2015

Thanks for the contribution! I will publish a new version in npm during the weekend.

@Munter
Copy link
Contributor Author

Munter commented Feb 20, 2015

Thanks 👍

@markusn
Copy link
Owner

markusn commented Feb 20, 2015

Tagged as 0.1.7 and published in npm.

@Munter
Copy link
Contributor Author

Munter commented Feb 20, 2015

Ping @sunesimonsen

@sunesimonsen
Copy link
Contributor

Great, thanks for the quick merge.

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

Successfully merging this pull request may close these issues.

4 participants