Opened 7 years ago

Closed 4 years ago

Last modified 4 years ago

#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 bill)

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)

dojo_require_imcompatiable_ie8.zip (2.3 KB) - added by split 5 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 7 years ago by Nathaniel Mills

I think to preserve the current function, the change should actually throw the caught error as in: add --> } catch(e){throw e; } finally

comment:2 Changed 7 years ago by Nathaniel Mills

ready.js has the same issue with a try/finally missing the catch

comment:3 Changed 7 years ago by bill

Component: GeneralCore
Owner: set to Nathaniel Mills
Status: newpending

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 7 years ago by Nathaniel Mills

Status: pendingnew

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 7 years ago by bill

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 7 years ago by Nathaniel Mills

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:7 Changed 7 years ago by bill

Yes, I understand that. My comment still stands.

comment:8 Changed 7 years ago by Nathaniel Mills

Yes, it would stop on the second throw, but the exception content remains the same.

comment:9 Changed 5 years ago by split

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 5 years ago by split

comment:10 Changed 5 years ago by split

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 5 years ago by dylan

Milestone: tbd1.11
Resolution: invalid
Status: newclosed

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

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

Resolution: invalid
Status: closedreopened

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

Priority: undecidedhigh

comment:15 Changed 4 years ago by dylan

Owner: changed from Nathaniel Mills to dylan
Status: reopenedassigned

comment:16 Changed 4 years ago by dylan

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

comment:17 Changed 4 years ago by Rawld Gill

comment:18 Changed 4 years ago by Dylan Schiemann <dylan@…>

Resolution: fixed
Status: assignedclosed

In a275e823/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:19 Changed 4 years ago by Dylan Schiemann <dylan@…>

In 37e8256/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:20 Changed 4 years ago by Dylan Schiemann <dylan@…>

In 430df9b/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:21 Changed 4 years ago by Dylan Schiemann <dylan@…>

In 16686d6/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:22 Changed 4 years ago by dylan

Milestone: 1.111.8.11
Note: See TracTickets for help on using tickets.