Bug Tracker

Ticket #9033 (closed bug: fixed)

Opened 4 years ago

Last modified 3 years ago

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
Blocking: Blocked by:

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

comment:1 follow-up: ↓ 4 Changed 4 years ago by timmywil

  • Owner set to azaozz <azaozz@…>
  • Priority changed from undecided to low
  • Status changed from new to pending
  • Component changed from unfiled to deferred

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 4 years ago by timmywil

  • Cc jaubourg added

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

  • Status changed from pending to new

Forgot to mention, the exact IE error is:

Exception thrown and not caught

comment:4 in reply to: ↑ 1 Changed 4 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 4 years ago by timmywil

  • Status changed from new to pending

comment:6 Changed 3 years ago by timmywil

#9246 is a duplicate of this ticket.

comment:7 follow-up: ↓ 8 Changed 3 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 3 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 3 years ago by misc@…

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

comment:10 Changed 3 years ago by arcticwalker

comment:11 Changed 3 years ago by anonymous

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

comment:12 Changed 3 years ago by timmywil

#9517 is a duplicate of this ticket.

comment:13 Changed 3 years ago by timmywil

  • Status changed from pending to closed
  • Resolution set to fixed

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

Changeset: 0a80be67f4fe968d99777564a02aeddbde1fbf35

comment:14 Changed 3 years ago by timmywil

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 3 years ago by timmywil

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone changed from 1.next to 1.7

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

comment:16 Changed 3 years ago by timmywil

  • Status changed from reopened to open

comment:17 Changed 3 years ago by timmywil

#9718 is a duplicate of this ticket.

comment:18 Changed 3 years ago by jaubourg

  • Owner changed from azaozz <azaozz@…> to jaubourg
  • Status changed from open to assigned

comment:19 Changed 3 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 3 years ago by jaubourg

#9878 is a duplicate of this ticket.

comment:21 Changed 3 years ago by rwaldron

#9993 is a duplicate of this ticket.

comment:22 Changed 3 years ago by rwaldron

#10030 is a duplicate of this ticket.

comment:23 Changed 3 years ago by rwaldron

#10162 is a duplicate of this ticket.

comment:24 Changed 3 years ago by rwaldron

#10181 is a duplicate of this ticket.

comment:25 Changed 3 years ago by rwaldron

  • Priority changed from low to blocker

comment:26 Changed 3 years ago by jaubourg

  • Status changed from assigned to closed
  • Resolution set to fixed

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 3 years ago by timmywil

#10348 is a duplicate of this ticket.

comment:28 Changed 3 years ago by rwaldron

#10428 is a duplicate of this ticket.

comment:29 Changed 3 years ago by timmywil

#10508 is a duplicate of this ticket.

comment:30 Changed 3 years ago by jaubourg

#11193 is a duplicate of this ticket.

comment:31 Changed 3 years ago by jaubourg

#11197 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.