241145 - nsIPrintSettings should not include nsFont.h

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect)

Description

[Marco Pesenti Gritti]

It's mentioned in http://www.mozilla.org/projects/embedding/apiReviewNotes.html
The more urgent problem is that nsFont.h drag in nsString.h. Currently it's not
possible to use embed strings with the print api.
I'll try provide a patch that removes it. The problem is that not building the
whole mozilla code there is a chance to break the build because of missing includes.

Comment 1

[Marco Pesenti Gritti]

Attachment: remove the unused include

Comment 2

[Marco Pesenti Gritti]

Caillon was suggesting to move the NS_GFX macros in gfx dir instead of adding
yet another nsComObsolete.h inclusion.
Similarly to what is done in msgCore.h, I was thinking to add a gfxCore.h with
#include "nscore.h" and the macros, then modify gfx headers to use it. I couldnt
find any other existing file suitable to host these macros...
Would this work ?

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 146642 [details] [diff] [review]
remove the unused include

Marco, I think that would be swell, though if you want this for 1.7 (which
seems both reasonable and likely), I say go with this patch on branch only, and
let's do the better fix for trunk.

r=caillon

Comment 4

[Marco Pesenti Gritti]

Comment on attachment 146642 [details] [diff] [review]
remove the unused include

Yeah it seem a good idea to get a fix in for 1.7 to me. The only risk is
possible include fallouts (it works ok on my linux gtk2 build but ...) though
tinderboxes should easily address that.

Comment 5

[Asa Dotzler [:asa]]

Comment on attachment 146642 [details] [diff] [review]
remove the unused include

a=asa (on behalf of drivers) for checkin to 1.7

Comment 6

[Marco Pesenti Gritti]

I dont have cvs access, so if someone could check it in I'd appreciate ;)

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

Landed on the 1.7 branch.

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

Note, I landed a bustage fix to
embedding/tests/mfcembed/components/PrintProgressDialog.cpp on the branch as well.

Comment 9

[Marco Pesenti Gritti]

Attachment: better fix for trunk

Comment 10

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 147446 [details] [diff] [review]
better fix for trunk

This looks good.  Do you also want to do widget while you're at it and kill off
nsComObsolete.h altogether?

Comment 11

[Marco Pesenti Gritti]

Yeah. There are a few more places where nsComObsolete.h is included according to
lxr, but I can deal with them.
Prolly we should try to get someone to test it on win32 too though, or we risk
some bad build bustage ...

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 147446 [details] [diff] [review]
better fix for trunk

Removing request for now.  Please come back when you figure out what you are
doing with widget, and testing on other platforms (find someone on #mozillazine
perhaps for platforms you do not have access to) would be nice.

Comment 13

[Marco Pesenti Gritti]

Attachment: Kill off nsComObsolete.h

I'll see if I can setup a windows build here and get someone to test on Mac.

Comment 14

[Marco Pesenti Gritti]

Comment on attachment 147714 [details] [diff] [review]
Kill off nsComObsolete.h

I tested this on linux gtk2 and windows mingw. I have not been able to find
someone to test on mac so far (will retry).

Comment 15

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 147714 [details] [diff] [review]
Kill off nsComObsolete.h

>-NS_BEGIN_EXTERN_C
>+#ifdef __cplusplus
>+extern "C" {
>+#endif

>-NS_END_EXTERN_C
>+#ifdef __cplusplus
>+}
>+#endif


All of these should become PR_{BEGIN|END}_EXTERN_C

The rest appears fine, but please clear it with dbaron and maybe even
blizzard/bryner.

Comment 16

[Marco Pesenti Gritti]

Ok, ccing blizzard and bryner. Maybe they have comment on the nsComObsolete.h
killoff. Will ask sr to dbaron.

Comment 17

[Marco Pesenti Gritti]

Attachment: use PR_BEGIN/END_EXTERN_C

Comment 18

[Marco Pesenti Gritti]

Attachment: Readd the build bustage fix I lost somewhere

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

attachment 148120 [details] [diff] [review] checked into trunk 2004-05-18 11:09 PDT.

Marco, is anything left to be done here?

Comment 20

[Marco Pesenti Gritti]

No, I think this is fixed now. Thanks!

Comment 21

[jag (Peter Annema)]

Do any of the compilers we still want to support not support anonymous namespaces?

Otherwise we should look (in a new bug, though) into replacing |static| with
|namespace { ... }| to hide symbols in a translation unit.