Ticket #6393 (reopened defect)

Opened 8 months ago

Last modified 3 weeks ago

[patch][cla] Existing setTimeout implementation overwritten by hostenv_rhino.js

Reported by: guest Owned by: dylan
Priority: normal Milestone: 1.3
Component: TestFramework Version: 1.1.0
Severity: normal Keywords:
Cc: jordi@…

Description

When running within Rhino, the hostenv_rhino.js file defines implementations of setTimeout and clearTimeout. However, the code doesn't check if those functions have already been defined. So those implementations will shadow prior definitions.

Other frameworks used in Rhino may define their own implementation of setTimeout, clearTimeout, setInterval, etc. It would be great if Dojo did not overwrite any existing implementation.

Examples of other implementations of setTimeout for Rhino are:

The main difference being whether they use Java threads or an event pump to behave more like the browsers which are single threaded.

- Jordi Albornoz Mulligan
jordi@…

Attachments

ticket6393.patch (1.7 kB) - added by guest 8 months ago.
Suggested patch - CLA submitted and acknowledged.

Change History

Changed 8 months ago by guest

Suggested patch - CLA submitted and acknowledged.

Changed 8 months ago by guest

Patch was attached by Jordi Albornoz Mulligan <jordi@…>.

Changed 5 months ago by dylan

  • status changed from new to assigned
  • owner changed from alex to dylan
  • milestone set to 1.2

Changed 5 months ago by dylan

  • summary changed from Existing setTimeout implementation overwritten by hostenv_rhino.js to [patch][cla] Existing setTimeout implementation overwritten by hostenv_rhino.js

Changed 5 months ago by dylan

  • status changed from assigned to closed
  • resolution set to fixed

(In [14299]) fixes #6393, prevent rhino from overriding any existing [set|clear]Timeout implementations, thanks Jordi Albornoz Mulligan for the patch, \!strict

Changed 2 months ago by Mark Wubben

  • status changed from closed to reopened
  • resolution deleted

In my current setup, resolving the setTimeout and clearTimeout variables does not throw an exception. Running Rhino 1.7.

I can't quite figure out why this is the case, but it wouldn't hurt adding:

	if(typeof(setTimeout) == 'undefined' || typeof(clearTimeout) == 'undefined'){ throw new Error; }

at the end of the try{} block.

Changed 2 months ago by Mark Wubben

In fact, this could replace the existing check, since the variables are referenced anyway.

Changed 2 months ago by peller

  • milestone changed from 1.2 to 1.3

Changed 3 weeks ago by dylan

Mark,

You mean like so:

Index: hostenv_rhino.js
===================================================================
--- hostenv_rhino.js    (revision 15698)
+++ hostenv_rhino.js    (working copy)
@@ -223,6 +223,7 @@
                                }
                                try{
                                        func();
+                                       if(typeof(setTimeout) == 'undefined' || typeof(clearTimeout) == 'undefined'){ throw new Error; }
                                }catch(e){
                                        console.debug("Error running setTimeout thread:" + e);
                                }

Note: See TracTickets for help on using tickets.