Last Comment Bug 241145 - nsIPrintSettings should not include nsFont.h
: nsIPrintSettings should not include nsFont.h
Status: RESOLVED FIXED
: fixed1.7
Product: Core Graveyard
Classification: Graveyard
Component: GFX (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Marco Pesenti Gritti
: Hixie (not reading bugmail)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-04-20 15:50 PDT by Marco Pesenti Gritti
Modified: 2009-01-22 10:17 PST (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
remove the unused include (995 bytes, patch)
2004-04-20 16:49 PDT, Marco Pesenti Gritti
caillon: review+
dbaron: superreview+
asa: approval1.7+
Details | Diff | Splinter Review
better fix for trunk (12.24 KB, patch)
2004-05-01 09:20 PDT, Marco Pesenti Gritti
no flags Details | Diff | Splinter Review
Kill off nsComObsolete.h (54.07 KB, patch)
2004-05-05 01:45 PDT, Marco Pesenti Gritti
no flags Details | Diff | Splinter Review
use PR_BEGIN/END_EXTERN_C (53.92 KB, patch)
2004-05-10 10:53 PDT, Marco Pesenti Gritti
no flags Details | Diff | Splinter Review
Readd the build bustage fix I lost somewhere (54.55 KB, patch)
2004-05-10 11:12 PDT, Marco Pesenti Gritti
caillon: review+
dbaron: superreview+
Details | Diff | Splinter Review

Description Marco Pesenti Gritti 2004-04-20 15:50:44 PDT
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 2004-04-20 16:49:06 PDT
Created attachment 146642 [details] [diff] [review]
remove the unused include
Comment 2 Marco Pesenti Gritti 2004-04-20 17:34:35 PDT
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) 2004-04-20 18:27:01 PDT
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 2004-04-21 00:57:51 PDT
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] 2004-04-28 16:22:52 PDT
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 2004-04-28 16:41:52 PDT
I dont have cvs access, so if someone could check it in I'd appreciate ;)
Comment 7 Christopher Aillon (sabbatical, not receiving bugmail) 2004-04-30 12:52:35 PDT
Landed on the 1.7 branch.
Comment 8 Christopher Aillon (sabbatical, not receiving bugmail) 2004-04-30 21:25:45 PDT
Note, I landed a bustage fix to
embedding/tests/mfcembed/components/PrintProgressDialog.cpp on the branch as well.
Comment 9 Marco Pesenti Gritti 2004-05-01 09:20:18 PDT
Created attachment 147446 [details] [diff] [review]
better fix for trunk
Comment 10 Christopher Aillon (sabbatical, not receiving bugmail) 2004-05-01 09:59:08 PDT
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 2004-05-01 17:04:32 PDT
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) 2004-05-05 00:53:35 PDT
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 2004-05-05 01:45:00 PDT
Created attachment 147714 [details] [diff] [review]
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 2004-05-09 05:27:38 PDT
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) 2004-05-10 07:14:07 PDT
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 2004-05-10 10:32:42 PDT
Ok, ccing blizzard and bryner. Maybe they have comment on the nsComObsolete.h
killoff. Will ask sr to dbaron.
Comment 17 Marco Pesenti Gritti 2004-05-10 10:53:46 PDT
Created attachment 148119 [details] [diff] [review]
use PR_BEGIN/END_EXTERN_C
Comment 18 Marco Pesenti Gritti 2004-05-10 11:12:17 PDT
Created attachment 148120 [details] [diff] [review]
Readd the build bustage fix I lost somewhere
Comment 19 Christopher Aillon (sabbatical, not receiving bugmail) 2004-05-18 11:10:23 PDT
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 2004-05-18 15:35:01 PDT
No, I think this is fixed now. Thanks!
Comment 21 jag (Peter Annema) 2004-05-18 16:36:33 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.