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)
Core
JavaScript Engine
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)
279 bytes,
text/html
|
Details | |
192 bytes,
text/plain
|
Details | |
3.69 KB,
patch
|
mrbkap
:
review+
dveditz
:
approval1.8.1.5+
|
Details | Diff | Splinter Review |
3.60 KB,
patch
|
dveditz
:
approval1.8.0.13+
|
Details | Diff | Splinter Review |
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+=" "+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," ")+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?
Reporter | ||
Comment 2•17 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•17 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•17 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•17 years ago
|
||
It is important to call the variable prototype and use the some argument of BUG in the builder.
Updated•17 years ago
|
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Comment 6•17 years ago
|
||
this alerts true in bonecho, minefield and false in ie7.
Reporter | ||
Comment 7•17 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•17 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•17 years ago
|
||
Assignee: general → brendan
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Updated•17 years ago
|
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha6
Assignee | ||
Comment 10•17 years ago
|
||
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•17 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•17 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•17 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
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•17 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•17 years ago
|
Status: REOPENED → ASSIGNED
Reporter | ||
Comment 14•17 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•17 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
Updated•17 years ago
|
Attachment #267086 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 16•17 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
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 17•17 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. :-)
Comment 18•17 years ago
|
||
This appears to have fixed a memory safety bug (bug 382503) so I think it should go into branch.
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.8.1.5?
Comment 19•17 years ago
|
||
/cvsroot/mozilla/js/tests/js1_5/Object/regress-382532.js,v <-- regress-382532.js initial revision: 1.1
Flags: in-testsuite+
Updated•17 years ago
|
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Comment 20•17 years ago
|
||
Brendan: will this patch work for the 1.8 branch or do you need to port something for it?
Assignee | ||
Comment 21•17 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 22•17 years ago
|
||
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•17 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•17 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•17 years ago
|
||
Attachment #274458 -
Flags: approval1.8.0.13?
Updated•17 years ago
|
Flags: blocking1.8.0.13?
Comment 26•17 years ago
|
||
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•17 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.
Description
•