Bug Tracker

Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#9033 closed bug (fixed)

try{ } finally{ } error in IE8

Reported by: azaozz <azaozz@…> Owned by: jaubourg
Priority: blocker Milestone: 1.7
Component: deferred Version: 1.5.2
Keywords: Cc: jaubourg
Blocked by: Blocking:

Description

Related #8478. Was trying to reopen it but that seems impossible.

Still seeing this in 1.5.2. On loading IE8 complains about missing catch statement on line 978 (in the try{ } finally{ } at line 973).

Ironically the other try{ } finally{ } (line 4493) has a catch(pseudoError){ } that avoids this in IE. Of course adding catch(pseudoError){ } on line 977 fixes the error.

If this is indeed invalid perhaps it should be removed from the other place too (line 4493). However this IE error seems triggered by quite a few scripts/plugins and I would suggest adding a catch(pseudoError){ } on line 978 and counting that as just another IE quirk that needs a minimal workaround.

Change History (31)

comment:1 Changed 12 years ago by Timmy Willison

Component: unfileddeferred
Owner: set to azaozz <azaozz@…>
Priority: undecidedlow
Status: newpending

Thanks for taking the time to contribute to the jQuery project! Please provide a reduced test case on http://jsFiddle.net that reproduces the issue experienced to help us assess your ticket. Additionally, test against the jQuery (edge) version to ensure the issue still exists.

I don't doubt this is a problem. I've seen many times where adding a dummy catch is necessary to avoid IE errors, but a test case to confirm would be helpful. L#57 of current deferred.js.

comment:2 Changed 12 years ago by Timmy Willison

Cc: jaubourg added

comment:3 Changed 12 years ago by azaozz <azaozz@…>

Status: pendingnew

Forgot to mention, the exact IE error is:

Exception thrown and not caught

comment:4 in reply to:  1 Changed 12 years ago by azaozz <azaozz@…>

Replying to timmywil:

...but a test case to confirm would be helpful.

Of course. If I can make the time this weekend I'll try to put a reduced test case on jsFiddle.

However there is a point about the difference in the two try{} finally{} statements currently in use. If one needs catch(pseudoError){} to avoid errors in IE, chances are the other would need that too, at least for consistency's sake.

comment:5 Changed 12 years ago by Timmy Willison

Status: newpending

comment:6 Changed 12 years ago by Timmy Willison

#9246 is a duplicate of this ticket.

comment:7 Changed 12 years ago by anonymous

Bug mentioned already here

http://webbugtrack.blogspot.com/2007/11/bug-184-catch-to-try-catch-finally-in.html

I get it in IE8 on XP in jQuery 1.6.1 unless I add a catch here:

// resolve with given context and args
resolveWith: function( context, args ) {
  if ( !cancelled && !fired && !firing ) {
    // make sure args are available (#8421)
    args = args || [];
    firing = 1;
    try {
      while( callbacks[ 0 ] ) {
        callbacks.shift().apply( context, args );
      }
    }
    catch(e) {}
    finally {
      fired = [ context, args ];
      firing = 0;
    }
  }
  return this;
},

comment:8 in reply to:  7 Changed 12 years ago by anonymous

Missed the message:

Unexpected call to method or property access.
Line: 5569
Char: 5
Code: 0
URI: http://code.jquery.com/jquery-1.6.1.js

comment:9 Changed 12 years ago by misc@…

does this still need a test case to move from pending?

comment:10 Changed 12 years ago by arcticwalker

comment:11 Changed 12 years ago by anonymous

Please, fix the bug. Please, notice that it is not a feature request.

comment:12 Changed 12 years ago by Timmy Willison

#9517 is a duplicate of this ticket.

comment:13 Changed 12 years ago by Timmy Willison

Resolution: fixed
Status: pendingclosed

Add catch block to try/finally in deferred. Fixes #9033. Test case needed.

Changeset: 0a80be67f4fe968d99777564a02aeddbde1fbf35

comment:14 Changed 12 years ago by Timmy Willison

Revert "Add catch block to try/finally in deferred. Fixes #9033. Test case needed." Line of exception was lost when debugging.

This reverts commit 0a80be67f4fe968d99777564a02aeddbde1fbf35.

Changeset: 39a2f29c29fdb296bb178b3c55d805ea0dc2f0ce

comment:15 Changed 12 years ago by Timmy Willison

Milestone: 1.next1.7
Resolution: fixed
Status: closedreopened

Fix was reverted, but I hear jaubourg has a fix for this when $.Callbacks is introduced in 1.7.

comment:16 Changed 12 years ago by Timmy Willison

Status: reopenedopen

comment:17 Changed 12 years ago by Timmy Willison

#9718 is a duplicate of this ticket.

comment:18 Changed 12 years ago by jaubourg

Owner: changed from azaozz <azaozz@…> to jaubourg
Status: openassigned

comment:19 Changed 12 years ago by azaozz

Looked at this further. The try{} finally{} seems to be triggered only on script parse errors. For example I had a stray comma at the end of an object definition (as you probably know FF and WebKit silently ignore these but IE throws an error on loading the script).

In that terms I'm not sure if the whole try{} finally{} is needed there as it may let unparsable scripts to go unnoticed. On the other hand jQuery silences most JS errors, so why not these...

comment:20 Changed 12 years ago by jaubourg

#9878 is a duplicate of this ticket.

comment:21 Changed 12 years ago by Rick Waldron

#9993 is a duplicate of this ticket.

comment:22 Changed 12 years ago by Rick Waldron

#10030 is a duplicate of this ticket.

comment:23 Changed 12 years ago by Rick Waldron

#10162 is a duplicate of this ticket.

comment:24 Changed 12 years ago by Rick Waldron

#10181 is a duplicate of this ticket.

comment:25 Changed 12 years ago by Rick Waldron

Priority: lowblocker

comment:26 Changed 12 years ago by jaubourg

Resolution: fixed
Status: assignedclosed

There is no try/finally block in the new $.Callbacks code. Deferred objects are "locked" if and when an exception occurs in a callback.

Related commit: https://github.com/jquery/jquery/commit/4092e3d2754e3847cd3159edb23184d4cfd4cf03

comment:27 Changed 12 years ago by Timmy Willison

#10348 is a duplicate of this ticket.

comment:28 Changed 12 years ago by Rick Waldron

#10428 is a duplicate of this ticket.

comment:29 Changed 12 years ago by Timmy Willison

#10508 is a duplicate of this ticket.

comment:30 Changed 11 years ago by jaubourg

#11193 is a duplicate of this ticket.

comment:31 Changed 11 years ago by jaubourg

#11197 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.