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

VERIFIED FIXED in mozilla1.9alpha6

Status

()

P1
normal
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: laszlo.janszky, Assigned: brendan)

Tracking

({fixed1.8.0.13, verified1.8.1.5})

unspecified
mozilla1.9alpha6
fixed1.8.0.13, verified1.8.1.5
Points:
---
Bug Flags:
blocking1.8.1.5 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
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>

Comment 1

11 years ago
??? why would someone want to use the micro$oft JScript FileSystemObject on mozillian browsers?
(Reporter)

Comment 2

11 years ago
(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.
(Reporter)

Comment 3

11 years ago
(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>
(Reporter)

Comment 4

11 years ago
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>
(Reporter)

Comment 5

11 years ago
It is important to call the variable prototype and use the some argument of BUG in the builder.

Updated

11 years ago
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general

Comment 6

11 years ago
Created attachment 266765 [details]
testcase

this alerts true in bonecho, minefield and false in ie7.
(Reporter)

Comment 7

11 years ago
(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:)
(Reporter)

Comment 8

11 years ago
here is the compactest code:

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

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

(Assignee)

Comment 9

11 years ago
Created attachment 267084 [details]
minimal combined browser/js-shell testcase
Assignee: general → brendan
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Updated

11 years ago
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha6
(Assignee)

Comment 10

11 years ago
Created attachment 267086 [details] [diff] [review]
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).

/be
Attachment #267086 - Flags: review?(mrbkap)
(Assignee)

Comment 11

11 years ago
(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
(Assignee)

Updated

11 years ago
Summary: Have a Core Javascript bug in functions: instanceof,isPrototypeOf. → instanceof, etc. broken by use of |prototype| in heavyweight constructor
(Reporter)

Comment 12

11 years ago
(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
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

11 years ago
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 → ---
(Assignee)

Updated

11 years ago
Status: REOPENED → ASSIGNED
(Reporter)

Comment 14

11 years ago
(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 :)
(Assignee)

Comment 15

11 years ago
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+
(Assignee)

Comment 16

11 years ago
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
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 17

11 years ago
(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. 
:-)

Updated

11 years ago
Blocks: 382503

Comment 18

11 years ago
This appears to have fixed a memory safety bug (bug 382503) so I think it should go into branch.
(Assignee)

Updated

11 years ago
Flags: blocking1.8.1.5?

Comment 19

11 years ago
/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?
(Assignee)

Comment 21

11 years ago
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+
(Assignee)

Comment 23

11 years ago
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

Comment 24

11 years ago
verified fixed 1.8.1, 1.9.0 windows/linux/macppc opt/debug browser/shell 7/16
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.5 → verified1.8.1.5

Comment 25

11 years ago
Created attachment 274458 [details] [diff] [review]
same for 1.8.0 branch
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+
(Assignee)

Comment 27

11 years ago
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.