Status

()

Core
XBL
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: sicking, Assigned: sicking)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

There is no reason for nsIBindingManager to be an XPCOM interface. It's almost exclusively used inside gkcontent and we're never going to freeze it.

Patch coming up that kills nsIBindingManager and nsIStyleRuleSupplier since the bindingmanager is the only implementation of both those interfaces.

I copied the few functions that existed outside of gkcontent to nsIDocument. This isn't ideal but I couldn't think of a better solution. And ideally some of this will be cleaned up as I wade through the XBL code.

I didn't make any changes to any call signatures as this patch is big enough as it is. I plan to do that in a separate patch.
Created attachment 255033 [details] [diff] [review]
Patch to fix

The patch to nsBindingManager.h is a bit messy. The only thing I did was copy the comments from nsIBindingManager.h and change the calling convention. I also removed the GetEnclosingScope function to avoid having to #include nsIContent.h
Attachment #255033 - Flags: superreview?(jst)
Attachment #255033 - Flags: review?(jst)
Attachment #255033 - Attachment is patch: true
Attachment #255033 - Attachment mime type: application/octet-stream → text/plain
Why not leave the member as nsRefPtr, or even nsCOMPtr given that we now only implement one interface?
r=dbaron on the mozilla/layout/style/* changes, although I'm a little puzzled by the nsInspectorCSSUtils.cpp #include change.
The #include is needed since nsIDocument.h no longer #includes ns(I)BindingManager.h. I didn't want to include nsBindingManager.h since that drags in stuff from other modules so I would have had to add to REQUIRES in various places.

This is also the reason why nsIDocument::mBindingManager can't be an nsRefPtr as the dtor is inlined. Is that the member you were referring to bz?
Yes, that's the one I meant.  Add a comment explaining that in the header, maybe?
Comment on attachment 255033 [details] [diff] [review]
Patch to fix

r+sr=jst, but the last two files (xul.css and nsCycleCollectionParticipant.h) are unrelated to this change, right?.
Attachment #255033 - Flags: superreview?(jst)
Attachment #255033 - Flags: superreview+
Attachment #255033 - Flags: review?(jst)
Attachment #255033 - Flags: review+
Opps, the xul.css change is unrelated yes. The nsCycleCollectionParticipant.h change is needed though.
This was checked in a while ago.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.