<< Back to previous view

[CLJS-847] TypeError in cljs.core/str using Safari 6.0.5 Created: 29/Aug/14  Updated: 31/Aug/14  Resolved: 31/Aug/14

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Kevin Neaton Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs-847.patch    

 Description   

In some versions of Safari 6 (at least 6.0.4 and 6.0.5) calls to cljs.core/str may throw a very cryptic Type Error: type error. This error has occurred repeatedly in our production cljs app over the last ~3 months but I have not been able to reproduce it locally, or boil it down to a simple test case. This appears to be due to the nature of the bug itself. I was however, able to workaround the issue by patching cljs.core/str to use the '' + x form of coercion instead of calling x.toString directly.

Other js projects have encountered this issue and adopted the same solution:

This shouldn't hurt performance and might actually improve it in some browsers:

I'll submit the patch we are using shortly.



 Comments   
Comment by David Nolen [ 31/Aug/14 9:41 AM ]

thanks for the report this is now fixed in master https://github.com/clojure/clojurescript/commit/08b4b1585cf0ef739e903985ee4c6b7fc6c47642.

Comment by Kevin Neaton [ 31/Aug/14 1:30 PM ]

My pleasure. Thanks for the quick turnaround!





[CLJS-846] Preserve namespace metada Created: 27/Aug/14  Updated: 28/Aug/14  Resolved: 28/Aug/14

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Joel Holdbrooks Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 846.patch    

 Description   

Namespace metadata is currently discarded and not available on cljs-ns/ns. Preserving this metadata is useful for some macros.



 Comments   
Comment by David Nolen [ 28/Aug/14 9:53 AM ]

fixed https://github.com/clojure/clojurescript/commit/edd35fa206d8285b2d671264347a1f0024631b13





[CLJS-839] Keyword hashing mobile safari (ios7) w/ advanced compilation Created: 11/Aug/14  Updated: 27/Aug/14  Resolved: 27/Aug/14

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Christian Karlsen Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File cljs-839.patch    

 Description   

I've been debugging some really funky behavior from mobile safari and found the root cause to be keyword hashing.

Version >= 2261 seems to be affected. Please see https://github.com/ckarlsen84/cljs-keyword-hashtest for a quick and dirty minimal test app (`lein ring server`).

Output from some of my runs: https://gist.github.com/ckarlsen84/4c16684c1f9e5ce26711

It seems to only happen with advanced compilation and mobile safari.



 Comments   
Comment by Thomas Heller [ 12/Aug/14 4:34 PM ]

Duplicate of CLJS-830. Nice to have a better example, was way too hard to reproduce in my app.

Comment by David Nolen [ 12/Aug/14 5:03 PM ]

This makes me think it's an operator precedence issue. But who knows, will look into it. Oh can we get details on the hardware? I was actually unable to repro this at least in Thomas Heller's case on an iPhone 5S running iOS 7.

Thanks for the minimal case!

Comment by Christian Karlsen [ 12/Aug/14 6:38 PM ]

I ran the tests with an iPhone4 (MC603KN/A).

Comment by David Nolen [ 18/Aug/14 9:54 AM ]

Christian, am I correct in interpreting the results as actually hashing incorrectly only after several initial runs?

Comment by David Nolen [ 21/Aug/14 5:37 PM ]

In order to investigate, this ticket needs more information. Specifically it would be useful to know if native Math.imul or the shim is being used and to confirm whether the behavior occurs only after multiple runs.

Comment by Christian Karlsen [ 22/Aug/14 2:15 AM ]

Only happens after multiple runs (usually after 10-12 runs iirc). I'm travelling atm but I'll do some more tests during the weekend.

Easiest way to test if i'm using the shim or not?

Christian

Comment by David Nolen [ 22/Aug/14 7:15 AM ]

We def a imul fn based on the presence and correctness of js/Math.imul. Just do imul.toString() or something like that to figure out what you have.

The multiple runs thing makes me suspect the bug only manifests when JavaScriptCore JIT (or more efficient interpreters) kick in. This would also explain why people can't repro with the debugger attached.

Comment by David Nolen [ 26/Aug/14 12:03 PM ]

I've attached a patch (which I am unable to test since I don't have a suitable device), it would be great if anyone encountering this issue could test the patch, thanks.

Comment by Thomas Heller [ 26/Aug/14 2:40 PM ]

I did not try the patch, but I used the imul.js file and just included it in my HTML before ANY other generated javascript. Which should have the same effect as the patch.

It seems to fix the issue, but I'm too tired to do a complete check. Will do a complete check tommorrow. Looks promising though.

Comment by Thomas Heller [ 27/Aug/14 3:14 AM ]

I had the fix deployed over night, but it seems clients are still sending me maps with duplicate keys which means it doesn't work.

Will investigate later.

Comment by Kris Jenkins [ 27/Aug/14 4:12 AM ]

I've tried this test-case on an iPhone 5 (MD299B/A), and it breaks as expected. With the patch against 4b81710, it works.

Comment by Kris Jenkins [ 27/Aug/14 5:06 AM ]

Yep, this patch also fixes the bug we were seeing on our production builds under 2311. Woo-hoo!

Comment by David Nolen [ 27/Aug/14 8:30 AM ]

Kris thanks for the feedback. Looking forward to hearing if Thomas Heller sees positive results after further investigation.

Comment by Thomas Heller [ 27/Aug/14 9:05 AM ]

So far I have not been able to reproduce the bug when the fix is applied.

I forgot to include the fix on some pages which explains why I was still getting invalid maps from Safari clients.

Will keep an eye on the log but so far it seems to be resolved.

Comment by David Nolen [ 27/Aug/14 9:37 AM ]

Thanks everyone for the help on this one, fixed in master and I cut a new release 0.0-2322 https://github.com/clojure/clojurescript/commit/3c0b8e71e09a971a07e3eef70bd74df6f1104d92

Comment by Kris Jenkins [ 27/Aug/14 9:44 AM ]

Fantastic! Thanks David.





Generated at Mon Sep 01 14:26:26 CDT 2014 using JIRA 4.4#649-r158309.