Closed Bug 382532 Opened 17 years ago Closed 17 years ago

instanceof, etc. broken by use of |prototype| in heavyweight constructor

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha6

People

(Reporter: laszlo.janszky, Assigned: brendan)

References

Details

(Keywords: fixed1.8.0.13, verified1.8.1.5)

Attachments

(4 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; hu; rv:1.8.0.11) Gecko/20070312 Firefox/1.5.0.11
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; hu; rv:1.8.0.11) Gecko/20070312 Firefox/1.5.0.11

I wrote this javascript code, but "alert(Type.isPrototypeOf(key))" and "key instanceof Type" doesn't work well. In Internet Explorer the code works well, and the result of "alert(Type.isPrototypeOf(key))" is "true", but in Firefox "false". I don't know why is that. So it's a Core Javascript Bug.

Reproducible: Always

Steps to Reproduce:
In Additional Information is the code.
Actual Results:  
"Type.isPrototypeOf(key)" returns "false"

Expected Results:  
Type.isPrototypeOf(key)" returns "true"

The javascript CODE:
<script>
Object.extend=function (o,e){if (o instanceof Object && e instanceof Object) for (var i in e) o[i]=e[i]; return o}
Function.prototype.parameters=function (){for (var i=0,a=[]; i<this.arguments.length; i++) a[i]=this.arguments[i];return a;}
Object.toString=function (o){var s=[];for (var i in o) s.push(i+" => "+o[i]);return s.join(", ");}
String.construct=function (times,iterator){var s="";for (i=0; i<times; i++) s+=iterator;return s}
Function.prototype.write=function (o){this.prototype=o; return this}
Function.prototype.extend=function (e){Object.extend(this.prototype,e); return this}
Object.merge=function (o,e){if (typeof o=="object" && typeof e=="object") for (var i in e) if (!(i in o)) o[i]=e[i]; return o}


function Model()
{
 var nodeTypes={};

 var prototype=
 {
  model: this,
  addChild: function ()
  {
   if (!this.hasChildren) return false;
   var parameters=arguments.callee.parameters(),type=parameters.shift();
   type=type instanceof Type?type:nodeTypes[type];
   if (!(type instanceof Type)) return false;
   this.childNodes.push(new type.setup(type,parameters,this));
   return this.childNodes[this.childNodes.length-1];
  },
  removeChild: function ()
  {

  }
 }


 function Type(key,addon,initialize)
 {
  this.key=key;
  this.prototype=Object.merge(addon,prototype);
  this.initialize=initialize;
  this.setup=function (type,parameters,parent)
  {
   this.nodeType=type;
   this.parentNode=parent;
   this.level=this.parentNode.level+1;
   if (this.hasChildren) this.childNodes=[];
   if (typeof initialize=="function") initialize.apply(this,parameters);
   return this
  }
  this.setup.prototype=addon;
 }


 var setup=function()
 {
  var parameters=arguments.callee.parameters();
  if (this.hasChildren) this.childNodes=[];
  this.level=1;
  alert(Object.toString(this.constructor.prototype))
  if (typeof this.nodeType.initialize=="function") this.nodeType.initialize.apply(this,parameters);
  return this;
 }

 Object.extend(setup,
 {
  addType: function (key,addon,initialize){nodeTypes[key]=new Type(key,addon,initialize)},
  removeType: function (key){delete nodeTypes[key]},
  rootType: function (key){key=key instanceof Type?key:nodeTypes[key]; alert(Type.isPrototypeOf(key));if (!(key instanceof Type)) return false;this.prototype=key.prototype;this.prototype.nodeType=key;}
 });

 return setup;
}

var FileSystemObject=new Model()
with (FileSystemObject)
{
 addType("root",{hasChildren: true,print: function (){var s="OBJECT TREE<br \/>"+this.name+"<br \/>";for (var i=0; i<this.childNodes.length; i++)s+="&nbsp;"+this.childNodes[i].print();return s}},function (name){this.name=name});
 rootType("root");
 addType("folder",{hasChildren: true,print: function (){var s=this.name+"<br \/>";for (var i=0; i<this.childNodes.length; i++)s+=String.construct(this.level,"&nbsp;")+this.childNodes[i].print();return s}},function (name){this.name=name});
 addType("file",{hasChildren: false,print: function (){return this.name+"<br \/>"}},function (name){this.name=name});
}

var FSO=new FileSystemObject("root");
 var alap=FSO.addChild("folder","alap");
  var mely=alap.addChild("folder","mély");
   mely.addChild("file","mély gyerek");
   mely.addChild("file","mély lány");
  var deep=alap.addChild("folder","deep");
   deep.addChild("file","deep girl");
 var ground=FSO.addChild("folder","ground");
  ground.addChild("file","ground girl");
 FSO.addChild("file","settings");
document.write(FSO.print())

var FSO2=new FileSystemObject("root2");
document.write(FSO2.print())

</script>
??? why would someone want to use the micro$oft JScript FileSystemObject on mozillian browsers?
(In reply to comment #1)
> ??? why would someone want to use the micro$oft JScript FileSystemObject on
> mozillian browsers?
> 

lol :D sure
it isnt the "micro$oft JScript FileSystemObject", just the same name.
(In reply to comment #1)
> ??? why would someone want to use the micro$oft JScript FileSystemObject on
> mozillian browsers?
> 

Okay, new code:

<script>


function TEST()
{
 var x;
 var prototype={}

 function BUG(i)
 {
  this.s=function ()
  {
   alert(i)
  }
  this.s.prototype=prototype;
 }

 this.test=function (){x=new BUG(111);alert("Is it bug?\n"+!(x instanceof BUG))}
}

var a=new TEST()
a.test();

</script>
newnewnewnew code :D

<script>
function TEST()
{
 var prototype={}

 function BUG(data)
 {
  this.builder=function ()
  {
   this.data=data;
  }
  this.builder.prototype=prototype;
 }

 this.test=function (){alert("Is it bug?\n"+!(new BUG(100) instanceof BUG))}
}

var a=new TEST()
a.test();
</script>
It is important to call the variable prototype and use the some argument of BUG in the builder.
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Attached file testcase
this alerts true in bonecho, minefield and false in ie7.
(In reply to comment #6)
> Created an attachment (id=266765) [details]
> testcase
> this alerts true in bonecho, minefield and false in ie7. 

yepp so ff bugged:)
here is the compactest code:

var prototype, x;
    
function Bug() {
    var func = function () {
        x;
    };
    prototype;
}

alert(
     "Bug: "+ !(new Bug instanceof Bug)
);

Assignee: general → brendan
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha6
Attached patch fixSplinter Review
Look out for other places where JSRESOLVE_HIDDEN should be used (global resolve hooks -- in which case JSRESOLVE_HIDDEN should go in jsapi.h).

/be
Attachment #267086 - Flags: review?(mrbkap)
(In reply to comment #10)
> Created an attachment (id=267086) [details]
> fix
> 
> Look out for other places where JSRESOLVE_HIDDEN should be used (global resolve
> hooks -- in which case JSRESOLVE_HIDDEN should go in jsapi.h).

My inspection found only function objects and call objects as the obj param to js_LookupHiddenProperty, but double-check me.

/be
Summary: Have a Core Javascript bug in functions: instanceof,isPrototypeOf. → instanceof, etc. broken by use of |prototype| in heavyweight constructor
(In reply to comment #11)
> (In reply to comment #10)
> > Created an attachment (id=267086) [details] [details]
> > fix
> > 
> > Look out for other places where JSRESOLVE_HIDDEN should be used (global resolve
> > hooks -- in which case JSRESOLVE_HIDDEN should go in jsapi.h).
> 
> My inspection found only function objects and call objects as the obj param to
> js_LookupHiddenProperty, but double-check me.
> 
> /be
> 

Thx for the fixation, i don't know the first thing about c, so i can't check you :>
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Don't resolve bugs as fixed for which you didn't land the patch. My comment was aimed at mrbkap, whose code review I'd requested.

/be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
(In reply to comment #13)
> Don't resolve bugs as fixed for which you didn't land the patch. My comment was
> aimed at mrbkap, whose code review I'd requested.
> 
> /be
> 

Ohh OKAY :)
László: thanks for finding this bug. The normal life-cycle for a patch is to get attached with r?reviewer@foo.com set to solicit code review, and after the patch is stamped r+, then it can be committed. Only then might the bug be marked FIXED by the person who committed the patch, or a designated helper.

Thanks again for this report and the help slimming down the testcase.

/be
Attachment #267086 - Flags: review?(mrbkap) → review+
Fixesd on trunk. This will show up in Firefox 3, but you would have to persuade the 1.8.1.x Gecko drivers to take it for Firefox 2.0.0.x to see it.

js/src/jsfun.c 3.185
js/src/jsobj.c 3.346
js/src/jsobj.h 3.64

/be
Status: ASSIGNED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
(In reply to comment #15)
> László: thanks for finding this bug. The normal life-cycle for a patch is to
> get attached with r?reviewer@foo.com set to solicit code review, and after the
> patch is stamped r+, then it can be committed. Only then might the bug be
> marked FIXED by the person who committed the patch, or a designated helper.
> 
> Thanks again for this report and the help slimming down the testcase.
> 
> /be
> 

Not at all! 
Me thanks, that i can take part in the development of firefox 3. 
:-)
Blocks: 382503
This appears to have fixed a memory safety bug (bug 382503) so I think it should go into branch.
Flags: blocking1.8.1.5?
/cvsroot/mozilla/js/tests/js1_5/Object/regress-382532.js,v  <--  regress-382532.js
initial revision: 1.1
Flags: in-testsuite+
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Brendan: will this patch work for the 1.8 branch or do you need to port something for it?
Comment on attachment 267086 [details] [diff] [review]
fix

It applies cleanly to the MOZILLA_1_8_BRANCH.

/be
Attachment #267086 - Flags: approval1.8.1.5?
Comment on attachment 267086 [details] [diff] [review]
fix

approved for 1.8.1.5, a=dveditz for release-drivers
Attachment #267086 - Flags: approval1.8.1.5? → approval1.8.1.5+
js/src/jsfun.c 3.117.2.31
js/src/jsobj.c 3.208.2.53
js/src/jsobj.h 3.39.4.10

/be
Keywords: fixed1.8.1.5
verified fixed 1.8.1, 1.9.0 windows/linux/macppc opt/debug browser/shell 7/16
Status: RESOLVED → VERIFIED
Attachment #274458 - Flags: approval1.8.0.13?
Flags: blocking1.8.0.13?
Comment on attachment 274458 [details] [diff] [review]
same for 1.8.0 branch

approved for 1.8.0.13, a=dveditz for release-drivers
Attachment #274458 - Flags: approval1.8.0.13? → approval1.8.0.13+
js/src/jsfun.c 3.117.2.7.2.15
js/src/jsobj.c 3.208.2.12.2.27
js/src/jsobj.h 3.39.4.3.2.5
Keywords: fixed1.8.0.13
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: