-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
$.each fails intermittently on iOS due to Safari bug #2145
Comments
Thanks for the report! This is really weird and due to being a timing issue it seems impossible to write a test for it. :( Did you report it to Apple? And/or to https://bugs.webkit.org/? We'd like an issue we'd be able to link to. I agree we should work around it. |
@mzgol I agree that writing a test case would be impossible. I didn't report the issue to Apple. I think it would carry more weight coming from the jQuery team, but if you point me to the right direction I'd be happy to file with them myself. |
The description you provided seems detailed enough. You could report a bug at https://bugs.webkit.org/, mention it affects jQuery & link to this issue. |
Link to WebKit bug: https://bugs.webkit.org/show_bug.cgi?id=142792 |
Wow. I guess this must be rare enough that it doesn't occur that often? Plain object, only numeric indices, etc. There's a patch in jashkenas/underscore#2094 that says it tries to work around this. We could do something similar but it seems like we're in the same situation as we were with the December release. Is it worth doing another minor-point release for this? |
I'd like to get a response to this WebKit bug first to know what we're dealing with. @BenjaminPoulain, could you have a look? |
Yep. This looks like a bug in one of the two optimizers. The problem likely only occurs after the code has been executed a few thousand times. First, I'll try to make a test case to reproduce. |
It might be... We could ask people affected to comment here. This seems like a bug that manifests mainly in large apps where it will be hard to debug. If Apple is going to fix it & backport a fix to the 8.x line we could punt on this but we won't know before the release (judging by past events) so IMO we should proceed assuming there won't be any backport here. |
I suggest this go out in the next point release regardless of what Apple does. 8.x will have a lot of market share for a while to come and this bug is VERY nasty to diagnose. It took me weeks to unearth the core problem. jQuery is in a great position to spare people the anguish. |
Yes, I meant that if Apple backported this fix to iOS 8.3 & Safari 8.0.5 then we shouldn't release a patch; before most people would have noticed the bug would be fixed. If the fix will get only to iOS 9/Safari 9 then we still need a patch since we'll be supporting iOS 8/Safari 8 for a while. I'd still want to hear from more affected people but I tend to agree this is something that will need a patch release. :/ |
BTW: according to jashkenas/underscore#2094 (comment):
we won't be able to write a meaningful test for it since we run the test in simulators as well. :/ Unless someone is going to run the suite manually on iOS 8 from time to time (the number of people being able to do it will drastically decrease with the release of iOS 9). |
I have had a hell of a time reproducing this. I cannot even tell if the bug exists in WebKit trunk or not. Any attempt at instrumenting the JIT makes the bug disappear due to the changes of timing. Has anyone found a reliable way to reproduce this bug? I have been using the test case of http://stackoverflow.com/questions/28155841/misterious-failure-of-jquery-each-and-underscore-each-on-ios but it is very fragile. |
The test case from jashkenas/underscore#2081 (comment) triggers the bug for me on an iPhone 5s with iOS 8.2 very reliably. I don't see the bug in the iOS 8.2 simulator. For posterity: var getLength = function(array) {
return array.length;
};
// JIT getLength with only arrays
for (var i = 0; i < 1000; i++) getLength([]);
alert(getLength({5: 'test'})); // => alerts 6 |
Note that the JIT is per-function, not per incorrectly treated object. The following code: var getLength = function(array) {
return array.length;
};
var getLength1 = function(array) {
return array.length;
};
var o = {5: 'test'};
// JIT getLength with only arrays
for (var i = 0; i < 1000; i++) getLength([]);
alert(getLength(o) + ', ' + getLength1(o)); will alert |
Per the meeting today, we'll be doing a hot-fix patch release for this. |
That is, if perf is not an issue. |
Seems like a good idea. |
@timmywil I let myself to create the milestone for the new patch releases & changed this one from 3.0.0 to the new one. We can always change back to 3.0.0 if we discover an issue with possible patches. |
@mzgol Sounds good. Thanks! |
+1 for the same issue. Looking forward jquery could cover this. |
I am swamped with other tasks at the moment, I have not had the chance to take a second look. As far as I can tell, the bug still exists. It is good you have a release addressing this. |
Michael had a look and fixed the bug: https://bugs.webkit.org/show_bug.cgi?id=142792 |
Awesome, and what a bug it was! http://trac.webkit.org/changeset/182058 I guess we don't have any idea of when it will be merged into the latest Safari or whether it will be back-ported, so that means a new release is still on. |
Actually I spoke too soon. It seems lodash users were mistaken in reporting the issue as resolved. |
FYI: this has not been fixed in iOS 8.3. |
In Chrome
Do I need to do something to avoid this? |
@yamau6809 This is a bug in DataTables code, please report an issue to them. Also, see #2242 for a previous report of this issue. |
@mzgol |
The fix has been committed to DataTables now and its nightly version is up to date with the change. Apologies for the error. |
Hi, I came across with this bug while writing iOS app using UIWebView with jQuery 2.1.4 (minified) hosted here. Is there available fixed version of jQuery? |
@nokuno This bug is fixed in jQuery 2.1.4. Maybe you experience a different issue? |
@mzgol You are right, the issue is not this one. Thank you! |
The fix for this bug causes error in Chrome, IE and Firefox. Tested with Chrome 44, 46; IE 11; and Firefox 38. Calling the function using an empty string as argument will reproduce the error. Note that this function was not called directly from a script, but from triggering a custom event on an element using element.trigger('eventName').
|
Can you open a separate ticket on the jQuery repo showing how this can happen when using the public API? |
Ignore my previous comment. After further investigation of the problem I found the cause of the problem. It was caused by improper public API use. In previous jQuery versions...
will fail silently. jQuery 2.1.4 will show that error. |
There is a timing bug in iOS8 that causes mobile Safari to incorrectly report a 'length' on objects that don't have one.
To the best of my knowledge, this happens on iOS8+, possibly only on 64-bit systems. The bug is triggered for objects that have only numeric properties. For example:
In this case, if you query
foo.length
then mobile Safari will sometimes return 4 (the highest property + 1).This causes functions like
$.each()
to treat objects such as foo above as arrays instead of objects, and when it tries to iterate them as such it fails since there is no foo[0].The problem can be fixed in the function
isArrayLike()
. Instead of just checking foryou also need to check for
The latter check is immune to the iOS bug.
I realize this is a fix just for one browser, but it's a browser with a very large user base.
You can see more background and some repro steps at the following Stack Overflow discussion:
http://stackoverflow.com/questions/28155841/misterious-failure-of-jquery-each-and-underscore-each-on-ios
The text was updated successfully, but these errors were encountered: