Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14406 closed defect (fixed)

[regression] dojo.eval no longer uses dojo.global

Reported by: Adam Peller Owned by: Rawld Gill
Priority: blocker Milestone: 1.7.2
Component: Core Version: 1.7.0
Keywords: Cc: Javier Pedemonte, ben hockey, James Burke
Blocked By: #14749 Blocking:

Description (last modified by Adam Peller)

Prior to Dojo 1.7, dojo.eval was defined as

                return d.global.eval ? d.global.eval(scriptFragment) : eval(scriptFragment);    // Object

Currently, dojo.eval does not go through the dojo.global property. evals occur in the base window, which is a regression if called from within a dojo.withGlobal block.

Change History (16)

comment:1 Changed 7 years ago by Adam Peller

monkey patching a call to doc.global.eval complains on webkit because the 'this' doesn't match the caller

http://perfectionkills.com/global-eval-what-are-the-options/

Last edited 7 years ago by Adam Peller (previous) (diff)

comment:2 Changed 7 years ago by Adam Peller

Description: modified (diff)

comment:3 Changed 7 years ago by bill

Milestone: tbd1.7.1

Marking for 1.7.1 for probable inclusion in 1.7/ branch, although will likely be bumped to 1.7.2 if it isn't fixed in the next 24 hours.

Note that although 1.7 intentionally made the loader's eval() work the way it does, but that doesn't mean that dojo.eval() has to change.

Last edited 7 years ago by bill (previous) (diff)

comment:4 Changed 7 years ago by bill

Milestone: 1.7.11.7.2

These didn't make it into the 1.7.1RC, so bumping them to 1.7.2 (as stated in the email I sent yesterday).

comment:5 Changed 7 years ago by Rawld Gill

Status: newassigned

Before I fix this, could I get feedback on the documentation from 1.6 and prior about dojo.eval that states:

"A legacy method created for use exclusively by internal Dojo methods. Do not use this method directly, the behavior of this eval will differ from the normal browser eval."

Is the documentation wrong? Or are we going to make dojo.eval part of the official API?

comment:6 Changed 7 years ago by bill

Cc: James Burke added

Well, that wording was introduced in [21437], probably out of regret for creating and exposing that function, and to discourage people from using it, but ISTM it's still a public method regardless. At least until 2.0.

comment:7 Changed 7 years ago by Colin Snover

Priority: highblocker

Bulk update of open ticket priorities.

comment:8 Changed 7 years ago by Rawld Gill

In [27764]:

reverted dojo.eval to mostly v1.6- behavior, with some improvment for ie; see #14406; refs #14406; !strict

comment:9 Changed 7 years ago by Rawld Gill

v1.6 eval had the weakness that it behaved differently in IE compared to other browsers and brought in a local scope when indirect eval was not supported. 1.7 made an attempt to improve the behavior, however some code has come to rely on the old behavior.

[27764] essentially punts the issue and reverts to 1.6- behavior, but does include a slight tweak that will make IE work mostly like other browsers that support indirect eval except that it won't return a value.

For browsers that do not support indirect eval, this is still a fairly defective implementation and this method should be avoided when targeting such platforms.

Fixed in trunk, keeping open until backported to 1.7.2

comment:10 in reply to:  1 Changed 7 years ago by Rawld Gill

Replying to peller:

Could you give this a try, particularly on IE, and see if it solves your use case?

Thanks!

comment:11 Changed 7 years ago by bill

I just ran the IE8 regression and got a failure in tests._base._loader.bootstrap:

     _AssertFailure: assertTrue('undefined') failed     ERROR IN: 		 function evalWorks(t){
			t.assertTrue(dojo.eval("(true)"));
			t.assertFalse(dojo.eval("(false)"));
		} FAILED test: evalWorks 0 ms

(I'm guessing it's from your change. Please take a look.)

comment:12 Changed 7 years ago by bill

Blocked By: 14749 added

(In #14749) Rawld, this started with [27764].

comment:3 Changed 7 years ago by Rawld Gill

In [27791]:

yet another attempt to fix defective dojo.eval without breaking current usages; fixes #14749; refs #14406; !strict

comment:4 Changed 7 years ago by Rawld Gill

[27791]implements dojo.eval so that is works correctly in non-IE environments.

IE environments are a problem because the only way to get eval to execute in the global scope is to use execScript. Unfortuneately, execScript does not return a value as is expected by some current usages of dojo.eval so it is not an option. Executing dojo.eval in the middle of a module gives a very polluted, not-global environment (this is what was done in v1.6-). Using the loader's technique won't work because of the scope protection issues with webkit that started this ticket.

The solution offered in [27791] offers the best known technique to execute eval in a near-global scope given the painful constraints and history of dojo.eval.

Tested on IE 9, 8, 6, Chromium 18, Opera 11.6; FF 10.

comment:5 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [27793]:

backport [27791]; fixes #14406; !strict

comment:6 Changed 7 years ago by Rawld Gill

In [27798]:

missed file that should have been in [27791]; refs #14406; !strict

Note: See TracTickets for help on using tickets.