Closed Bug 310742 Opened 17 years ago Closed 17 years ago

Properties of DOM objects are listed twice or 3 times


(Core :: DOM: Core & HTML, defect, P1)

1.8 Branch





(Reporter: bugzilla, Assigned: brendan)




(Keywords: fixed1.8, regression)


(3 files)

Steps to reproduce:
1- Load provided URL
a) Click the "Attribute and methods of the DOCUMENT.BODY object" button
b) Click the "Attribute and methods of the EVENT object" button

Actual results in Seamonkey 1.1a rv: 1.9a1 build 2005100105 and Firefox 1.4.1
rv: 1.8b5 build 20051001:
a) document.object attribute and methods are listed twice
b) event attribute and methods are listed 3 times

Expected results:
Attributes and methods should be listed once and only once.

Reproducible: Always

- this must be a recent regression since I tried that demo not less than 2 weeks
- this could be a JS Engine component bug but I doubt it since the bug does not
happen for other objects
- if you try the DOCUMENT only button, you'll see that addEventListener is
listed twice

Reduced testcase coming up.
Attached file Testcase
Instructions: click the [Attributes and methods of the EVENT object ...] button

Expected results: only ABORT, ALT_MASK and AT_TARGET should be listed once and
only once.
Perhaps caused by bug 308856?

Last working version is Firefox 1.4 20050930
Confirmed regression on Firefox 1.4.1 20051001 (mac)
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Version: Trunk → 1.8 Branch
Backing out the 3 patches for bug 308856 seems to fix the problem... assuming I
applied the patches correctly. (But I was able to verify the issue without
patching then reverse patching and the testcase only shows 1 of each again.)
I'll look into this Monday at the latest.
Assignee: general → mrbkap
Flags: blocking1.8b5?
Blocks: 310864
blake, can you keep us up to date on your progress here? Time is very short for
this release.
This was caused, specifically, by attachment 197494 [details] [diff] [review]. We are now not ignoring
properties that we were ignoring before (apparently, rightly). I'm still not
sure where these other properties are coming from, however.
I spoke to Brendan about this. The original patch to fix this problem was bogus
(and thus caused this bug). This patch fixes the issue without the regressions
but is an awful hack. Brendan said he'd think about this (and hopefully come up
with a better solution). I'm attaching this in case we don't find a better one
in the short-term (i.e., for the branch).
Taking from mrbkap so he can focus on WinXP drop shadow fixage.

Assignee: mrbkap → brendan
Flags: blocking1.8b5? → blocking1.8b5+
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
This is must-fix for Firefox 1.5 and other Gecko rv:1.8 apps.

No need for a goto and new label.  Note also that given the current JS API in
full, we can't fix this bug except by making the outerObject thing the special
case that makes this enumeration work.

In other words, if we don't elaborate the old logic here (!prop || obj != obj2,
particularly the latter), we don't enumerate split windows.  If we elaborate it
to allow any obj2 not on obj's prototype chain, we fix the window enumeration
case but get multiple id values enumerated per property for
XPCWrappedNativeProto cases.  The exact case we want is therefore peculiar to
split windows, which are the only implementors of JSExtendedClass.outerObject.

Attachment #198385 - Flags: review?(mrbkap)
Attachment #198385 - Flags: approval1.8b5?
Comment on attachment 198385 [details] [diff] [review]
pretty, gentle hack

Ah, much better! r=mrbkap
Attachment #198385 - Flags: review?(mrbkap) → review+
Comment on attachment 198385 [details] [diff] [review]
pretty, gentle hack

Self-approving, this is needed by the same token as 308856, and it's specific
to split window, so no risk to other objects, and easy to prove a fix for the
window case.

Attachment #198385 - Flags: approval1.8b5? → approval1.8b5+
Passes testcase, checked into trunk, hitting branch next.

Fixed on branch and trunk.

Closed: 17 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Verified. Testcase passed. (mac build)
No longer blocks: 310864
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.