Closed
Bug 866225
Opened 12 years ago
Closed 12 years ago
remove some useless #includes from nsINode/Element/etc
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: smaug, Assigned: smaug)
Details
Attachments
(1 file)
5.03 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter 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)
Assignee | ||
Comment 1•12 years ago
|
||
Seems to compile locally
Comment 2•12 years ago
|
||
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+
Assignee | ||
Comment 3•12 years ago
|
||
(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.
Comment 4•12 years ago
|
||
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...
Assignee | ||
Comment 5•12 years ago
|
||
Comment 6•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•