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.

Sign up for GitHub

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

Jump to bottom

NumberControl with arrowButtonOptions or sliderOptions crashes on IE11 #493

Closed
samreid opened this issue Apr 29, 2019 · 5 comments
Closed

NumberControl with arrowButtonOptions or sliderOptions crashes on IE11 #493

samreid opened this issue Apr 29, 2019 · 5 comments
Assignees
@pixelzoom
Labels
status:ready-for-review

Comments

@samreid
Copy link
Member

samreid commented Apr 29, 2019

Discovered in phetsims/circuit-construction-kit-common#485, Array.includes is undefined on IE11, so any NumberControl with arrowButtonOptions or sliderOptions crashes on IE11. This problem looks like it was introduced in f5250b3 around Feb 2, 2019.
samreid added a commit that referenced this issue Apr 29, 2019
@samreid
Use _.includes instead of Array.includes, see #493
8d35d4b
@samreid
Copy link
Member Author

samreid commented Apr 29, 2019

I pushed a proposed fix, @mbarlow12 can you please review?

@samreid samreid mentioned this issue Apr 29, 2019
Closed
@pixelzoom
Copy link
Contributor

This is the tip of the iceberg. There are many other places in PhET code where Array.includes is used, including core classes like Screen, Enumeration, ComboBox,... How has this not been caught in testing?

@samreid
Copy link
Member Author

samreid commented Apr 30, 2019

For example, Screen.js usage of Array.includes is only in an assertion.

@pixelzoom pixelzoom mentioned this issue Apr 30, 2019
Closed
@pixelzoom
Copy link
Contributor

Moving the general issue of replacing Array.includes with _.includes to phetsims/tasks#998.

@pixelzoom
Copy link
Contributor

For example, Screen.js usage of Array.includes is only in an assertion.

Then that tells me that we're not testing with assertions on IE. Why?!?

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

@pixelzoom pixelzoom

Labels
status:ready-for-review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@samreid @pixelzoom @ariel-phet @mbarlow12