Closed Bug 866225 Opened 9 years ago Closed 9 years ago

remove some useless #includes from nsINode/Element/etc

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Still compiling locally, but should be ok, since this is just removing includes which
are there anyway because of some other header file.
Well, except nsIDOMEvent.h

https://tbpl.mozilla.org/?tree=Try&rev=fc5ef93d5739
Attachment #742484 - Flags: review?(continuation)
Seems to compile locally
Comment on attachment 742484 [details] [diff] [review]
patch

Review of attachment 742484 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nsINode.h added back to nsIContent.h.  You are doing a little bootlegging otherwise, but just for grandparent classes or omnipresent Gecko classes, so it sounds reasonable to me.

::: content/base/public/Element.h
@@ -37,5 @@
>  #include "nsIInlineEventHandlers.h"
>  #include "mozilla/CORSMode.h"
>  #include "mozilla/Attributes.h"
>  #include "nsContentUtils.h"
> -#include "nsINodeList.h"

I do see nsIDOMNodeList in here, maybe that should be at least forward declared?

::: content/base/public/nsIContent.h
@@ -10,2 @@
>  #include "nsIDocument.h"     // for use in inline function (IsInHTMLDocument)
> -#include "nsINode.h"         // for base class

Please leave this #include for the direct base class in place.  Otherwise we're just picking it up indirectly via something that says "for use in inline function" which seems bad.
Attachment #742484 - Flags: review?(continuation) → review+
(In reply to Andrew McCreight [:mccr8] from comment #2)
> I do see nsIDOMNodeList in here, maybe that should be at least forward
> declared?
Don't understand this. nsINodeList.h is in FragmentOrElement.h

> ::: content/base/public/nsIContent.h
> @@ -10,2 @@
> >  #include "nsIDocument.h"     // for use in inline function (IsInHTMLDocument)
> > -#include "nsINode.h"         // for base class
> 
> Please leave this #include for the direct base class in place.  Otherwise
> we're just picking it up indirectly via something that says "for use in
> inline function" which seems bad.
Naturally we pick up nsINode.h from nsIDocument.h because nsIDocument is nsINode.
If you update the comment to mention that we're getting our base class from there or just delete the comment entirely I guess I'm okay with it...
https://hg.mozilla.org/mozilla-central/rev/ab534b63501a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.