If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Pass nsIStyledContent to nsIDocumentObserver::AttributeChanged

RESOLVED FIXED in mozilla1.9alpha1



12 years ago
12 years ago


(Reporter: bz, Assigned: bz)


(Blocks: 1 bug, {perf})


Firefox Tracking Flags

(Not tracked)



(1 attachment)

This will help us avoid a QI in nsCSSFrameConstructor and not affect any other
Created attachment 200419 [details] [diff] [review]
Like so.
Attachment #200419 - Flags: superreview?(jst)
Attachment #200419 - Flags: review?(bugmail)
Blocks: 313207
Comment on attachment 200419 [details] [diff] [review]
Like so.

The added include shouldn't be needed in:

>Index: extensions/transformiix/source/xpath/nsXPathResult.cpp
>Index: extensions/transformiix/source/xslt/txMozillaXSLTProcessor.cpp
>Index: docshell/shistory/src/nsSHEntry.cpp

with that r=me
Attachment #200419 - Flags: review?(bugmail) → review+
I'm not strongly against this, but wouldn't it make more sense to simply get rid of nsIStyledContent all together and move all its methods to nsIContent? Now we essentially require that all elements implement nsIStyledContent to play nice in the document observer world, why not simply say that all nsIContent classes need to expose what's currently in nsIStyledContent? 
We could do that, sure.  sicking, dbaron, what do you think?
I was considering that solution too. I don't really have a strong oppinion either way, but I also don't think we should add more stuff to nsIContent unless we actually gain something from it. I.e. if it would be a performance of code-cleanness benefit then it sounds like a good idea, if not I don't mind keeping nsIStyledContent and renamed it nsIElement
That said, I've been pondering merging nsITextContent into nsIContent :)

And looking at the interface it's almost all about various attribute things and we have most attribute stuff on nsIContent anyways...
So I really don't care either way.  Let's just pick something and then I'll implement it...
Comment on attachment 200419 [details] [diff] [review]
Like so.

I do still think that this would be cleaner if we'd simply eliminated nsIStyledContent since now we're making impossible for nsIContent implementations that are not nsIStyledContent from sending AttributeChanged notifications. Not that such classes exist (where it matters), but still, it seems a bit odd to require implementing this interface to be able to tell observers about an attribute change.

Wanna file a followup bug to change that?

sr=jst for this change in any case though...
Attachment #200419 - Flags: superreview?(jst) → superreview+
Filed bug 313968 on eliminating nsIStyledContent, and landed this.
Last Resolved: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
You need to log in before you can comment on or make changes to this bug.