Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#1218 closed defect (fixed)

Extending String with dojo.extend breaks String.toString() in IE

Reported by: organicveggie@… Owned by: Tom Trenka
Priority: high Milestone:
Component: String Version: 0.3
Keywords: Cc:
Blocked By: Blocking:

Description

If you extend the built-in String class using dojo.extend, it breaks the String class in Internet Explorer by overwriting the original String.toString with Object.toString.

Example:

dojo.lang.extend(String, { mytest: function() { } });

The problem occurs in source:trunk/src/bootstrap1.js in dojo._mixin(), which has an explicit block that handles custom toString methods in IE:

// IE doesn't recognize custom toStrings in for..in
if(dojo.render.html.ie && dojo.lang.isFunction(props["toString"]) && 
   props["toString"] != obj["toString"])

Assuming the user did not add a custom toString method, the property object passed in automatically inherits a toString() method from Object. In the test above, propstoString? is the same as Object.toString() and will never match objtoString? because the String prototype overrides toString().

In my opinion, the appropriate fix is to add an additional test against Object.toString:

dojo._mixin = function(/*Object*/ obj, /*Object*/ props){
    // summary:     Adds all properties and methods of props to obj.
    var tobj = {};
    for(var x in props){
        // the "tobj" condition avoid copying properties in "props"
        // inherited from Object.prototype.  For example, if obj has a custom
        // toString() method, don't overwrite it with the toString() method
        // that props inherited from Object.protoype
        if(typeof tobj[x] == "undefined" || tobj[x] != props[x]) {
            obj[x] = props[x];
        }
    }
    // IE doesn't recognize custom toStrings in for..in
    if(dojo.render.html.ie && dojo.lang.isFunction(props["toString"]) && 
       props["toString"] != obj["toString"]) && 
       props["toString"] != tobj["toString"]) { // <-- ADDED test against tobj
        obj.toString = props.toString;
    }
    return obj;
}

Attachments (1)

dojo._mixin.diff (566 bytes) - added by organicveggie@… 13 years ago.
Diff against trunk at r5100

Download all attachments as: .zip

Change History (6)

Changed 13 years ago by organicveggie@…

Attachment: dojo._mixin.diff added

Diff against trunk at r5100

comment:1 Changed 13 years ago by organicveggie@…

Version: 0.20.3

Whoops, forget to specify this was the 0.3x series. Reproduced against trunk and the patch was against trunk at revision 5100.

comment:2 Changed 13 years ago by dylan

Component: GeneralString
Milestone: 0.4
Owner: changed from anonymous to psowden

comment:3 Changed 13 years ago by Tom Trenka

Owner: changed from psowden to Tom Trenka

I'll take it, will fix shortly.

comment:4 Changed 13 years ago by Tom Trenka

Resolution: fixed
Status: newclosed

Fixed, r5267

comment:5 Changed 12 years ago by (none)

Milestone: 0.4

Milestone 0.4 deleted

Note: See TracTickets for help on using tickets.