Closed
Bug 677101
Opened 13 years ago
Closed 13 years ago
Various #include improvements
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
9.56 KB,
patch
|
tnikkel
:
review+
Ms2ger
:
checkin+
|
Details | Diff | Splinter Review |
82.17 KB,
patch
|
mounir
:
review+
|
Details | Diff | Splinter Review |
24.33 KB,
patch
|
mounir
:
review+
|
Details | Diff | Splinter Review |
884 bytes,
patch
|
mounir
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Flags: in-testsuite-
Assignee | ||
Comment 1•13 years ago
|
||
This is necessary because nsAutoLayoutPhase uses nsContentUtils, which I don't want to include in nsPresContext.h
Attachment #551327 -
Flags: review?(tnikkel)
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #551328 -
Flags: review?(mounir)
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #551329 -
Flags: review?(mounir)
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #551330 -
Flags: review?(mounir)
Comment 5•13 years ago
|
||
So what is the goal here?
Assignee | ||
Comment 6•13 years ago
|
||
Not including things unnecessarily, and in particular not including nsContentUtils.h from headers, because it includes a lot of stuff itself.
Comment 7•13 years ago
|
||
Comment on attachment 551327 [details] [diff] [review] Part a: Create nsAutoLayoutPhase.h >diff --git a/layout/base/nsAutoLayoutPhase.h b/layout/base/nsAutoLayoutPhase.h >+ >+#ifndef nsAutoLayoutPhase_h >+#define nsAutoLayoutPhase_h >+ >+#include "nsPresContext.h" >+#include "nsContentUtils.h" >+ >+#ifdef DEBUG Move the "#ifdef DEBUG" above the other includes? r+ assuming this is basically a straight copy of code from one place to another.
Attachment #551327 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 8•13 years ago
|
||
Will do.
Comment 9•13 years ago
|
||
Comment on attachment 551328 [details] [diff] [review] Part b: Remove nsContentUtils.h includes from headers Review of attachment 551328 [details] [diff] [review]: ----------------------------------------------------------------- Do whatever you want with my comments (they are only suggestions). r=mounir ::: content/svg/content/src/nsSVGLength2.h @@ +175,5 @@ > NS_IMETHOD SetValue(float aValue) > { > + if (!NS_finite(aValue)) { > + return NS_ERROR_ILLEGAL_VALUE; > + } Maybe add a comment explaining that you don't use NS_ENSURE_FINITE to prevent including nsContentUtils.h? @@ +186,5 @@ > NS_IMETHOD SetValueInSpecifiedUnits(float aValue) > { > + if (!NS_finite(aValue)) { > + return NS_ERROR_ILLEGAL_VALUE; > + } Same here. ::: content/svg/content/src/nsSVGNumber2.h @@ +112,5 @@ > NS_IMETHOD SetBaseVal(float aValue) > { > + if (!NS_finite(aValue)) { > + return NS_ERROR_ILLEGAL_VALUE; > + } and here.. ::: content/svg/content/src/nsSVGNumberPair.h @@ +121,5 @@ > NS_IMETHOD SetBaseVal(float aValue) > { > + if (!NS_finite(aValue)) { > + return NS_ERROR_ILLEGAL_VALUE; > + } oh, here too :)
Attachment #551328 -
Flags: review?(mounir) → review+
Comment 10•13 years ago
|
||
Comment on attachment 551329 [details] [diff] [review] Part c: Reduce nsIDOMText.h inclusions Review of attachment 551329 [details] [diff] [review]: ----------------------------------------------------------------- I believe you checked that nothing in the includes you removed were actually used? r=mounir assuming that.
Attachment #551329 -
Flags: review?(mounir) → review+
Comment 11•13 years ago
|
||
Comment on attachment 551330 [details] [diff] [review] Part d: Remove some includes from nsHTMLInputElement.cpp Review of attachment 551330 [details] [diff] [review]: ----------------------------------------------------------------- r=mounir
Attachment #551330 -
Flags: review?(mounir) → review+
Assignee | ||
Comment 12•13 years ago
|
||
Comment on attachment 551327 [details] [diff] [review] Part a: Create nsAutoLayoutPhase.h http://hg.mozilla.org/mozilla-central/rev/36989c74b287
Attachment #551327 -
Flags: checkin+
Assignee | ||
Comment 13•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/f4537a268e6f http://hg.mozilla.org/mozilla-central/rev/eeec7694bf6a http://hg.mozilla.org/mozilla-central/rev/134ee27f55a7
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Assignee | ||
Updated•11 years ago
|
Blocks: includehell
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•