Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#10235 closed defect (fixed)

fileuploader breaks when used in a page with moo tools

Reported by: ben hockey Owned by: Mike Wilcox
Priority: low Milestone: 1.4
Component: DojoX Form Version: 1.3.2
Keywords: Cc:
Blocked By: Blocking:

Description

there is a 'helper' function in fileuploader called mixin which fails when moo tools is in the same page. the issue is due to the extra properties in the Array prototype added by moo tools and then the mixin function inspects arrays/objects via something like:

for(var p in arr){
    // do stuff 
}

this is obviously not dojo's fault but can we work around it?

Change History (6)

comment:1 Changed 10 years ago by dante

Owner: changed from dante to Mike Wilcox

... i'd wondered what the mixin function did over d.mixin ...

comment:2 Changed 10 years ago by Mike Wilcox

@neonstalwart: can you offer a patch, or at least some kind of solution? If so I can apply it right away, otherwise, it may take time to get to this.

comment:3 Changed 10 years ago by ben hockey

for some reason it fails on properties which are constructors. so the following pseudo-code would fix the first loop.

for(var nm in o1){
    // if o1[nm] is an object and NOT a function then mixin the rest of the properties
    // else assign the property.
}

and something similar is required for the 2nd loop. i haven't thoroughly tested it so i'm not going to claim that it's comprehensive but it does fix my case.

i've also found some strange behavior that i'm not sure is intentional.

var foo = {a: 'aaa'};
var bar = {b: 'bbb', c:foo};
var baz = {d: 'ddd'};
var bat = mixin(baz, bar);

/* 
--> bat == {
    b: 'bbb',
    d: 'ddd'
} 
*/
var foo = {a: 'aaa'};
var bar = {b: 'bbb', c:foo};
var baz = {d: 'ddd'};
var bat = mixin(bar, baz);

/* 
--> bat == {
    b: 'bbb',
    d: 'ddd',
    c: {
        a: 'aaa'
    }
} 
*/

@phiggins (dante) it seems that one of the differences with dojo.mixin is that this function will do a 'deep' mixin where the properties at all levels are copied rather than referenced whereas dojo.mixin does not recurse into properties that are objects and so those properties will reference the same object(s) as referred to by the properties of the original object being mixed. also, this function returns a new object containing the combined properties without changing either of the 2 objects being mixed.

var foo = {a: 'aaa'}; 
var bar = {b: 'bbb', c:foo}; 
var baz = {d: 'ddd'};  
var bat = dojo.mixin(bar, baz);

foo.a = 'foo'; 
// --> bar.c.a == 'foo'
// --> bat.c.a == 'foo'
bat == bar // true
var foo = {a: 'aaa'};
var bar = {b: 'bbb', c:foo};
var baz = {d: 'ddd'};
var bat = mixin(bar, baz);

foo.a = 'foo';
// --> bar.c.a == 'foo'
// --> bat.c.a == 'aaa'
bat == bar // false

the question still remains as to whether all of this is necessary. the one time that this function gets called, i'm not sure why it wouldn't work to use dojo.mixin

return mixin(o, getTextStyle(node));

both of the objects passed to mixin were created from empty objects. so at first look, it doesn't look like we would break anything to mix one into the other and it also seems that just copying object properties by reference (rather than recursing into them to copy them by value) shouldn't break anything either. this would need to be tested though and i can't say whether it would address my specific case since i haven't taken a thorough look at how moo tools has messed things up.

for now, i would prefer to wait until any changes can be tested rather than apply a change based on some flawed logic. i don't think i can do much more to this anytime soon and i won't expect anyone to do anything i wouldn't do myself :)

comment:4 Changed 10 years ago by Mike Wilcox

Thanks for the suggestion. You went into a lot of detail, but I gather its just the custom mixin that breaks it? If so, that will be committed. Good catch that it wasn't doing much - I had started with the need for it and as things progressed it wasn't needed at all any more.

comment:5 Changed 10 years ago by Mike Wilcox

Resolution: fixed
Status: newclosed

(In [20771]) Fixes #10264, #10271, #10313, #10235, #10295 - works in Dialog boxes (still not tabs, that one is harder) - fixed HTML uploader issues. Added new properties to prevent errors of large uploads. Added ability for individual file errors (in Flash).

comment:6 Changed 10 years ago by Mike Wilcox

Milestone: tbd1.4
Note: See TracTickets for help on using tickets.