Closed
Bug 316990
Opened 19 years ago
Closed 19 years ago
window.hasOwnProperty('functionname') broken by split windows
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha1
People
(Reporter: bc, Assigned: mrbkap)
References
()
Details
(Keywords: regression, verified1.8.0.1, verified1.8.1)
Attachments
(1 file, 1 obsolete file)
1.41 KB,
patch
|
brendan
:
review+
dveditz
:
approval1.8.0.1+
dveditz
:
approval1.8.1+
|
Details | Diff | Splinter Review |
when running in the browser ecma_3/Function/scope-001.js and ecma_3/Function/scope-002.js fail section G with Expected value 'false,true', Actual value 'false,false' while they pass in the shell. function f() {} window.hasOwnProperty('f') is true in 1.7.12, but false for 1.8 and 1.9 since bug 296639.
Comment 1•19 years ago
|
||
Is this something that we should be fixing on the 1.8 branch?
Comment 2•19 years ago
|
||
Yes, this ought to be fixed. If we had noticed this sooner (thanks bc for noticing it at all!) we would have fixed it long ago. One consolation: at top level, the workaround for the example in comment 0 is just hasOwnProperty('f'). That should work (please say if it does not). /be
Flags: blocking1.8.1+
Flags: blocking1.8.0.1?
Assignee | ||
Comment 3•19 years ago
|
||
This could probably be fixed in a similar fashion to bug 310742 (which, IMO, makes sense for this case).
OS: Windows XP → All
Hardware: PC → All
Assignee | ||
Comment 4•19 years ago
|
||
Assignee | ||
Comment 5•19 years ago
|
||
Comment on attachment 203539 [details] [diff] [review] Wrong file. Oops, this is the wrong file.
Attachment #203539 -
Attachment description: Something like this → Wrong file.
Attachment #203539 -
Flags: review?(brendan)
Assignee | ||
Comment 6•19 years ago
|
||
Attachment #203539 -
Attachment is obsolete: true
Attachment #203540 -
Flags: review?(brendan)
Assignee | ||
Updated•19 years ago
|
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha
Reporter | ||
Comment 7•19 years ago
|
||
(In reply to comment #2) > One consolation: at top level, the workaround for the example in comment 0 is > just hasOwnProperty('f'). That should work (please say if it does not). function f() {} hasOwnProperty('f') false
Comment 8•19 years ago
|
||
Comment on attachment 203540 [details] [diff] [review] Something like this Do this stuff: >+ clasp = OBJ_GET_CLASS(cx, obj); >+ xclasp = (clasp->flags & JSCLASS_IS_EXTENDED) >+ ? (JSExtendedClass *)clasp >+ : NULL; After a brace and indent step following this else: >+ } else if (xclasp && xclasp->outerObject && >+ xclasp->outerObject(cx, obj2) == obj) { >+ *rval = JSVAL_TRUE; > } else if (OBJ_IS_NATIVE(obj2)) { > sprop = (JSScopeProperty *)prop; > *rval = BOOLEAN_TO_JSVAL(SPROP_IS_SHARED_PERMANENT(sprop)); > } else { > *rval = JSVAL_FALSE; > } Moving everything to here over. r=me with that. /be
Attachment #203540 -
Flags: review?(brendan) → review+
Comment 9•19 years ago
|
||
(In reply to comment #7) > (In reply to comment #2) > > One consolation: at top level, the workaround for the example in comment 0 is > > just hasOwnProperty('f'). That should work (please say if it does not). > > function f() {} > hasOwnProperty('f') > false D'oh. Our |this| binding code outerizes the inner object in whicih hasOwnProperty is found. Of course. This bug has no workaround. /be
Assignee | ||
Comment 10•19 years ago
|
||
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 11•19 years ago
|
||
verified fixed on cvs debug trunk builds 2005-11-23 on windows/linux.
Status: RESOLVED → VERIFIED
Updated•19 years ago
|
Flags: blocking1.8.0.1? → blocking1.8.0.1+
Comment 12•19 years ago
|
||
Comment on attachment 203540 [details] [diff] [review] Something like this a=dveditz for drivers
Attachment #203540 -
Flags: approval1.8.1+
Attachment #203540 -
Flags: approval1.8.0.1+
Reporter | ||
Comment 14•19 years ago
|
||
verified 2005-01-11 builds on windows/linux/mac on 1.8.0.1, 1.8, trunk
Assignee | ||
Comment 15•17 years ago
|
||
This caused bug 375344 because the OBJ_GET_CLASS was on the wrong object (oops!).
Depends on: 375344
Assignee | ||
Comment 17•17 years ago
|
||
This is covered in the JS test found in the URL field for this bug (ecma_3/Function/scope-001.js).
Comment 18•17 years ago
|
||
Do those run as part of the automated Tinderbox tests?
Assignee | ||
Comment 19•17 years ago
|
||
Sadly, no, they don't. IIRC, the js testsuite takes at least 30 minutes to run, even if one doesn't include the "long running" tests. bc has more info, I'm sure!
Comment 20•17 years ago
|
||
I don't think needing 30 mins to run is a problem, per se... Especially not if it's got a dedicated machine.
Reporter | ||
Comment 21•17 years ago
|
||
two-midairs! third time is the charm. (In reply to comment #18) > Do those run as part of the automated Tinderbox tests? > No. It appears the required work to get buildbot to manage running the js tests and reporting to a tinderbox has a lower priority than other testing efforts and may not happen any time soon. Perhaps not before Firefox 3 ships if I understood correctly. Until then, I run them across a variety of machines and try to keep up with mrbkap, brendan, igor, crowder. There was a period that I was not running the tests regularly while I worked on a method for dealing with known failures and reporting regressions, but I am back to running the tests on daily basis again. The shell tests, which would not have found this bug, actually run fairly quickly (depending on how you define "quickly"), but 30 minutes is definitely an upper bound. The 1.8.1 branch runs slower since it has more failures. The browser tests (which found this bug) take a couple of hours on a real machine to complete a run since they perform the following cycle for each test: start the browser, run the test, then shutdown. For the tests running on a VM, it can take considerably longer. To do a full test run for a branch, which includes opt, debug for shell, browser it can take on the order of a day on some of the VMs. In order to manage the results, I try to run the tests across all machines after brendan goes to sleep and before igor wakes up ;) and compare all of the results at once after the run completes. When these are managed by buildbot, I think we can limit the build types to debug builds, run the js shell tests frequently during the day, and run the browser tests once a day. I can also trim the timeout I am currently using and exclude more of the slow running tests to improve turn around time.
Flags: in-testsuite? → in-testsuite+
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•