Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#5334 closed defect (fixed)

Testing for "instanceof Array" does not work across page boundaries (iframe-to-iframe)

Reported by: guest Owned by: anonymous
Priority: high Milestone: 1.3
Component: General Version: 0.9
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Tom Trenka)

Because each page has a global context with its own "Array" function, if code on one page passes an array to a function on a separate page, the test "array instanceof Array" will fail. Thus, a Dojo API that behaves differently for array-valued parameters will do the wrong thing in that case.

Change History (13)

comment:1 Changed 11 years ago by dylan

Milestone: 1.3

comment:2 Changed 11 years ago by Tom Trenka

Description: modified (diff)
Resolution: invalid
Status: newclosed

Not sure what or why this ticket was filed; Internet Explorer (at least) does not allow you to pass objects by reference across window boundaries as it is.

This just sounds more like a JS+DOM gotcha, and less a real issue with anything in Dojo--since all intended actions that happen cross-window-boundary are tested before release to ensure they are working as advertised.

Closing this.

comment:3 Changed 10 years ago by Mike Wilson

I'd say that this report refers to the use of instanceof in isArray(), which in turn is used in many places in the dojo codebase:

dojo.isArray = function(/*anything*/ it){
  return it && (it instanceof Array || typeof it == "array"); // Boolean
}

I guess not so many are using multi-frame or multi-window designs nowadays, but the people who are will be hitting problems with the above. We've had users with this problem in DWR and have abandoned instanceof altogether. For Array we do this instead:

return it && Object.prototype.toString.call(it)=="[object Array]"

I'm not sure what Tom refers to but the problem is "real" when you do calls between windows passing array data to a function in another Document, and that function in turn uses something in Dojo that queries dojo.isArray.

comment:4 Changed 10 years ago by Mike Wilson

Resolution: invalid
Status: closedreopened

comment:5 in reply to:  4 ; Changed 10 years ago by Kris Zyp

Object.prototype.toString.call breaks on an array subclasses, so it's not a universal solution either. There is no universal solution, and one shouldn't rely on dojo.isArray for it. If users need to handle multiple Array constructors (for multi-frame envs) with code that uses dojo.isArray, they can monkey patch dojo.isArray with a Object.prototype.toString.call based function. We could discuss adding this as an option where a user could choose a djConfig option to use alternate dojo.isArray handling. I will document the limitations of dojo.isArray in the docs.

comment:6 Changed 10 years ago by Kris Zyp

Resolution: fixed
Status: reopenedclosed

Documentation updated at http://docs.dojocampus.org/dojo/isArray. Marking as fixed. We can reopen it as an enhancement if we want to add an option for an alternate handler, but this should *not* be default.

comment:7 in reply to:  5 Changed 10 years ago by Mike Wilson

Replying to kzyp:

Object.prototype.toString.call breaks on an array subclasses

By Ecma-262 it is not possible to subclass Array, and therefore isArray() only needs to concern instances of the native Array class. Could you please define what you mean with an Array subclass?

comment:8 Changed 10 years ago by Kris Zyp

By subclass, I meant in the prototype sense: CustomArray? = function(){}; CustomArray?.prototype = []; var customArrayInstance = new CustomArray?(); customArrayInstance instanceof Array -> true Object.prototype.toString.call(customArrayInstance)=="[object Array]" -> false

The toString method is also about twice as slow as the instanceof method. For most of my applications I would certainly prefer the instanceof method for isArray. It would be pretty presumptuous to assume users want the toString method. Note that isArrayLike should work fine for cross-frame arrays.

comment:9 Changed 10 years ago by Mike Wilson

With your example, the .length property breaks completely in IE and also breaks truncation in other browsers. Nobody uses it - f ex Dojo avoids it in NodeList?. Dean Edwards even resorts to using iframes to have his array subclasses: http://dean.edwards.name/weblog/2006/11/hooray/

Performance: yes, I have also measured the performance, see:
http://www.nabble.com/RE:-Parameter-marshalling-between-iframes-%2B-DWR-p18179220.html[[BR]] and the result was going from 11 usec (instanceof) to 17 usec (Object toString). It is safe to say that it is not this difference that will make your application unusable.

If not doing cross-frame support in isArray I see no reason to keep this utility in the library. If the only thing it is doing is |instanceof| then that's better done by client code itself.
(I have never come across the other case, typeof=="array", which I wonder if it's a typo or something from a long-gone unsupported browser?).

comment:10 Changed 10 years ago by Kris Zyp

IMO, this function doesn't have much utility to the user, the behavior is only really important because it is used internally extensively, which is another reason why it would be preferable to avoid changing it.

Of course, not all users necessarily need IE. Some users may have FF only environments or Rhino, so it is not always necessary to resort to iframes to subclass array.

typeof=="array" must have been someones wistful hope of browsers changing typeof behavior. Of course it will never happen. This is old code.

comment:11 Changed 10 years ago by Mike Wilson

If the isArray() method isn't providing any added value then I think it should be removed. A plain "instanceof Array" is known to all JS programmers, is clearer, and only three bytes longer.

If wanting it for internal use it should be renamed to _isArray.

In any case I think the typeof=="array" stuff should be removed.

comment:12 Changed 10 years ago by dante

it can't just "go away", we're stuck with it until at least 2.0 when this can be readdressed. one advantage of having a dojo.isArray is that (should you so choose) you are able to override it easily:

dojo.isArray = function(it){ .toString(stuff) }

and not need to change any your code or Dojo to work with it.

comment:13 Changed 10 years ago by bill

(In [20600]) Add comment that dojo.isArray() doesn't work across page boundaries (on IE). Refs #5334 !strict.

Note: See TracTickets for help on using tickets.