Properties of DOM objects are listed twice or 3 times

VERIFIED FIXED in mozilla1.8beta5

Status

()

Core
DOM: Core & HTML
P1
normal
VERIFIED FIXED
12 years ago
9 years ago

People

(Reporter: Gérard Talbot, Assigned: brendan)

Tracking

({fixed1.8, regression})

1.8 Branch
mozilla1.8beta5
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
Steps to reproduce:
1- Load provided URL
2- 
a) Click the "Attribute and methods of the DOCUMENT.BODY object" button
or
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

Note: 
- this must be a recent regression since I tried that demo not less than 2 weeks
ago.
- 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.
(Reporter)

Comment 1

12 years ago
Created attachment 198181 [details]
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)
Status: UNCONFIRMED → NEW
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?

Updated

12 years ago
Blocks: 310864

Updated

12 years ago
Blocks: 308856

Comment 5

12 years ago
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.
Created attachment 198376 [details] [diff] [review]
Ugly, brutal, hack

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).
(Assignee)

Comment 8

12 years ago
Taking from mrbkap so he can focus on WinXP drop shadow fixage.

/be
Assignee: mrbkap → brendan
Flags: blocking1.8b5? → blocking1.8b5+
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta5
(Assignee)

Comment 9

12 years ago
This is must-fix for Firefox 1.5 and other Gecko rv:1.8 apps.

/be
Status: NEW → ASSIGNED
(Assignee)

Comment 10

12 years ago
Created attachment 198385 [details] [diff] [review]
pretty, gentle hack

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.

/be
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+
(Assignee)

Comment 12

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

/be
Attachment #198385 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 13

12 years ago
Passes testcase, checked into trunk, hitting branch next.

/be
(Assignee)

Comment 14

12 years ago
Fixed on branch and trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Verified. Testcase passed. (mac build)
Status: RESOLVED → VERIFIED
No longer blocks: 310864

Updated

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