Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Make all nsINodeLists (except maybe the XBL one) inherit from nsWrapperCache

RESOLVED FIXED in mozilla6

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bz, Assigned: peterv)

Tracking

Trunk
mozilla6
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
I hear tell there's a partial fix for this.
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 1

6 years ago
Created attachment 525542 [details] [diff] [review]
v1
(Assignee)

Comment 2

6 years ago
Comment on attachment 525542 [details] [diff] [review]
v1

This seems to pass on tryserver.
Attachment #525542 - Flags: review?(bzbarsky)
(Reporter)

Comment 3

6 years ago
nsAnonymousContentList should unlink mContent.

Can you please catch me tomorrow?  I'd like to understand the exact behavior of parent stuff here, and in particular why it's ok for nsBaseContentList to QI to nsWrapperCache without reporting a useful parent.
(Reporter)

Comment 4

6 years ago
Ah, the key is that we need the parent to ensure only one wrapper, right?  Seems like nothing ensures that right now for nsBaseContentList, so shouldn't the QI to nsWrapperCache live on nsSimpleContentList?
(Reporter)

Comment 5

6 years ago
Or would that fail due to inheriting from nsWrapperCache anyway?

After this patch, do we still instantiate nsBaseContentList?  If so, do we only do it in places that won't get JS-wrapped or something?
(Assignee)

Comment 6

6 years ago
(In reply to comment #5)
> After this patch, do we still instantiate nsBaseContentList?

We do not, should be impossible because its GetParentObject is pure virtual (inherited from nsINodeList). I opted for that because there was only one remaining use of it (which also wasn't exposed to JS), I think it's simpler now.
Having the QI for nsWrapperCache live on nsBaseContentList is just convenience, I can instead add it to all the derived classes if you think that makes more sense (might be a tiny bit faster too).
The GetParentObject implementations all return a sensible parent I think, let me know if you agree :-).
(Reporter)

Comment 7

6 years ago
Comment on attachment 525542 [details] [diff] [review]
v1

> We do not, should be impossible because its GetParentObject is pure virtual

Aha, perfect.  I'd missed that.

r=me with the comment 3 bit about unlinking, then.
Attachment #525542 - Flags: review?(bzbarsky) → review+
(Reporter)

Comment 8

6 years ago
Peter, can you get this updated and checked in?  I'd really like to make the nodelist binding code depend on this patch....
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/mozilla-central/rev/623563df5bd1
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.