#16617 closed defect (fixed)
[patch][cla][pr] dojo.js guardCheckComplete has try / finally without catch, breaking IE8 compatibility
Reported by: | Nathaniel Mills | Owned by: | Rawld Gill |
---|---|---|---|
Priority: | high | Milestone: | 1.8.11 |
Component: | Core | Version: | 1.8.3 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
The release notes in 1.8.1 talk about IE8 compatibility, but my application breaks as soon as dojo.js is loaded because it has a try / finally without a catch in the guardCheckComplete method. The fix is simply to add an empty catch as shown below highlighted by add --> bold:
guardCheckComplete = function(proc){ try{ checkCompleteGuard++; proc(); add --> }catch(e){ }finally{ checkCompleteGuard--; } if(execComplete()){ signal("idle", []); } },
I looked in the 1.8.3 and latest dojo.js source and it also has this problem.
Attachments (1)
Change History (23)
comment:1 Changed 8 years ago by
comment:3 Changed 8 years ago by
Component: | General → Core |
---|---|
Owner: | set to Nathaniel Mills |
Status: | new → pending |
IE8 works fine for me, and as far as I know, no one else has this problem that you mention.
How do you reproduce it?
comment:4 Changed 8 years ago by
Status: | pending → new |
---|
I tripped across this because html fragments attempted to be brought into the js file using dojo.text! were not being retrieved so the html was null and caused the exception in guardCheckComplete. When I searched the web, I found several references to IE8 not supporting try / finally without catch. One example is here: http://stackoverflow.com/questions/5023674/jquery-throws-ie-specific-error If you search on try finally ie8 you can see other references to this problem. Another reference is here: http://stackoverflow.com/questions/8846471/have-you-had-problems-with-dojo-1-7-1-in-internet-explorer-7-8 I realize that the real problem is our own code causing the exception to occur, but the result is misleading because IE8 debugger stops on the error (try without a catch) and we can't see the exception that occurred. It would be better to catch the exception and throw it so it bubbles up to help people understand the problem they have created.
When I tried to reproduce today by creating an error in the html, I'm getting a different error: Expected ":" just past the word finally when the code was commented:
guardCheckComplete = function(proc){ try{ checkCompleteGuard++; proc(); // } catch(e) {throw e; } finally { // wnm3: fixed for IE8 checkCompleteGuard--; } if(execComplete()){ signal("idle", []); } },
To reproduce, you have to have something that triggers an exception to be thrown in the guardCheckComplete method. Unfortunately, my code has devolved to the point where I can't easily get it back to the original situation that brought this error to light. What I did today was just cause an error in the html by referencing a non-existing dijit class and commented out the catch clause.
comment:5 Changed 8 years ago by
Description: | modified (diff) |
---|
I see. Although I worry that adding the catch(e) workaround will make debugging harder on other platforms, especially when "pause on uncaught exceptions" (only) is set, because then won't chrome only stop on the second throw, rather than the original one?
comment:6 Changed 8 years ago by
Thanks Bill. Please note that I'd updated the suggested fix to include a throw e; statement after catching the exception so the exception would still exist and the break on uncaught exceptions would be triggered as I don't believe the method is called in an outer try/catch environment.
The add line should read: } catch (e) { throw e; } finally { ...
comment:8 Changed 8 years ago by
Yes, it would stop on the second throw, but the exception content remains the same.
comment:9 Changed 6 years ago by
I have simular issue on IE8. I created a custom widget, and then require[<my widget>]. But function guardCheckComplete will always throw an exception every "first" time on the page.
However, it works fine if I try require[<my widget>] again on the same web page. Please see my attachment.
https://bugs.dojotoolkit.org/attachment/ticket/16617/dojo_require_imcompatiable_ie8.zip
Changed 6 years ago by
Attachment: | dojo_require_imcompatiable_ie8.zip added |
---|
comment:10 Changed 6 years ago by
BTW, I found that the root cause is dojo.connect. If the old-style dojo.connect() is used in my widget, dojo.js will throw exception during requiring.
comment:11 Changed 6 years ago by
Milestone: | tbd → 1.11 |
---|---|
Resolution: | → invalid |
Status: | new → closed |
Makes sense, since you're not pulling in that dependency (if you pulled in "dojo" as a dependency that would have worked, though that's not really what you want to do anyway)...
comment:12 Changed 6 years ago by
Hi guys, inherited an old site with a commitment to support IE8 and whose codebase depends heavily on dojo 1.9.1. I'm having difficulty understanding the rationale in closing this – reading between the lines I'm assuming some variation of "user error/won't fix/not a bug". Although looking at the timestamps, it could well be "OP moved on in life".
@split do you remember solution?
comment:13 Changed 6 years ago by
Resolution: | invalid |
---|---|
Status: | closed → reopened |
Hmm, it sounds like split's problem was that he didn't specify his dependencies correctly. But AFAICT the root issue still stands, so I guess we should reopen this.
comment:14 Changed 5 years ago by
Priority: | undecided → high |
---|
comment:15 Changed 5 years ago by
Owner: | changed from Nathaniel Mills to dylan |
---|---|
Status: | reopened → assigned |
comment:16 Changed 5 years ago by
Owner: | changed from dylan to Rawld Gill |
---|---|
Summary: | dojo.js guardCheckComplete has try / finally without catch, breaking IE8 compatibility → [patch][cla][pr] dojo.js guardCheckComplete has try / finally without catch, breaking IE8 compatibility |
PR needs review at https://github.com/dojo/dojo/pull/199 .
comment:22 Changed 5 years ago by
Milestone: | 1.11 → 1.8.11 |
---|
I think to preserve the current function, the change should actually throw the caught error as in: add --> } catch(e){throw e; } finally