Permalink
Browse files

Fixes #8353. Adds a catch block in resolveWith so that the finally bl…

…ock gets executed in IE7 and IE6.
  • Loading branch information...
1 parent e405419 commit cacea6f7e778d42cda56066a6b1da8fb163410cc @jaubourg jaubourg committed Feb 23, 2011
Showing with 6 additions and 0 deletions.
  1. +6 −0 src/core.js
View
@@ -843,6 +843,12 @@ jQuery.extend({
callbacks.shift().apply( context, args );
}
}
+ // We have to add a catch block for
@dmethvin
dmethvin Feb 23, 2011 Member

Should this be done only for IE<9 perhaps? It will make debugging difficult for ajax/ready/deferred code.

@jaubourg
jaubourg Feb 23, 2011 Member

Can we use @cc_on now? I mean, I seem to remember it was removed because of Google Closure, does it work now with uglify?

@dmethvin
dmethvin Feb 23, 2011 Member

Looks like uglify also strips comments before it processes so I think the answer is still "no."

I tried writing a feature test for this, but the act of nesting an outer catch causes the inner finally block to execute. So conditional comments are probably our only hope. http://jsfiddle.net/dmethvin/vvPDB/

It does seem like this will cause headaches in debugging though, no?

@jaubourg
jaubourg Feb 23, 2011 Member

It's weird, tests seem to indicate try/finally works in IE6: http://jsfiddle.net/gCme4/2/

Unfortunately, I can't test IE7 here.

Do we know of any "recent" update to IE6 and/or IE7 that could have fixed it?

@dmethvin
dmethvin Feb 23, 2011 Member

I think your test case is working for the same reason as mine, because the outer catch fixes things. If there is just a try/finally in the execution chain the finally is never executed.

@rkatic
rkatic Feb 24, 2011 Contributor

to delete

@rkatic
rkatic Feb 24, 2011 Contributor

So, now in IE6/7 (without the catch block) in case exceptions are not caught, next done() calls will NOT resume resolving. In others will. Why not to resume (halted) resolving by a resumeResolving() and not by done()? In that way we can remove try/finally blocks completely. I am still VERY suspicious about the usefulness of the (inpredictable) "auto resume feature". Have you any case where it is useful and still correct? Related discussion: http://forum.jquery.com/topic/deferred-and-resolve

@dmethvin
dmethvin Feb 24, 2011 Member

rkatic, can you post a full code example of your proposal in the forum thread?

@rkatic
rkatic Feb 24, 2011 Contributor

@dmethvin: well it's already there in post http://forum.jquery.com/topic/deferred-and-resolve#14737000002013797 as "Solution D" (_Deferred code at https://gist.github.com/828650). Hope it is clear enough.

+ // IE prior to 8 or else the finally
+ // block will never get executed
+ catch (e) {
+ throw e;
+ }
finally {
fired = [ context, args ];
firing = 0;

0 comments on commit cacea6f

Please sign in to comment.