phetsims / phet-core

Core utilities used by all PhET simulations.
MIT License
8 stars 6 forks source link

Array.reverse broken on iOS 12 and Mac Safari 12 #41

Closed samreid closed 5 years ago

samreid commented 6 years ago

See https://github.com/fanmingfei/array-reverse-ios12. The webkit bug page says a push has been committed, but we have no way of knowing when Apple will roll this out: https://bugs.webkit.org/show_bug.cgi?id=188794

We have approximately 60 usages of .reverse() across our repos. I have not updated to iOS 12 or Mac Safari 12 so I haven't checked the sims to see this bug in action. We should decide how to proceed.

jonathanolson commented 6 years ago

Having QA test (and being informed of places where reverse() is called) might be a good option.

jbphet commented 6 years ago

How about we also help QA determine whether a given platform has the issue? For instance, we could create a simple page that would print whether the problem exists, so that QA can see first of all if it exists on many of our devices and second which sims break in the presence of the bug.

pixelzoom commented 6 years ago

What @jbphet said. But start with a simple page that only tests Array.reverse. Then if it fails, put in the additional effort to determine which sims break.

samreid commented 6 years ago

The link in the initial comment provides a test page and the linked stackoverflow page has another (clearer?) test page. It looks like refreshing the browser is part of the process required to reproduce the problem.

pixelzoom commented 6 years ago

FYI, iOS 12 and Safari 12 were released on 9/17/18. Safari 12 is supported on macOS 10.12 - 10.14.

samreid commented 6 years ago

@jonathanolson pointed out that this only affects arrays of primitives, not arrays of objects. @ariel-phet will check in with the QA team to make sure they are testing on iOS 12 and Mac Safari 12 in the next RCs.

ariel-phet commented 6 years ago

Have QA test on iOS 12.0 with a known problematic safari version next week on all published sims (1 min test) if possible.

ariel-phet commented 5 years ago

Closing this issue, we will see what the QA test reveals.

samreid commented 5 years ago

Would it be better to leave this issue open and on hold until testing complete?

ariel-phet commented 5 years ago

My plan was to open a new issue with relevant information, and the plan to address it IF anything is identified by QA, it is unclear if this is even a problem for us.

Closing, and please leave closed @samreid