The default bug view has changed. See this FAQ.

Various #include improvements

RESOLVED FIXED in mozilla8

Status

()

Core
DOM
--
trivial
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug)

Trunk
mozilla8
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 551327 [details] [diff] [review]
Part a: Create nsAutoLayoutPhase.h

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

6 years ago
Created attachment 551328 [details] [diff] [review]
Part b: Remove nsContentUtils.h includes from headers
Attachment #551328 - Flags: review?(mounir)
(Assignee)

Comment 3

6 years ago
Created attachment 551329 [details] [diff] [review]
Part c: Reduce nsIDOMText.h inclusions
Attachment #551329 - Flags: review?(mounir)
(Assignee)

Comment 4

6 years ago
Created attachment 551330 [details] [diff] [review]
Part d: Remove some includes from nsHTMLInputElement.cpp
Attachment #551330 - Flags: review?(mounir)
So what is the goal here?
(Assignee)

Comment 6

6 years ago
Not including things unnecessarily, and in particular not including nsContentUtils.h from headers, because it includes a lot of stuff itself.
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

6 years ago
Will do.
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 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 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

6 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

6 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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
(Assignee)

Updated

4 years ago
Blocks: 785103
You need to log in before you can comment on or make changes to this bug.