nsIPrintSettings should not include nsFont.h

RESOLVED FIXED

Status

Core Graveyard
GFX
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: Marco Pesenti Gritti, Assigned: Marco Pesenti Gritti)

Tracking

({fixed1.7})

Trunk
x86
Linux
fixed1.7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
Created attachment 146642 [details] [diff] [review]
remove the unused include
(Assignee)

Comment 2

13 years ago
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 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
Attachment #146642 - Flags: review+
(Assignee)

Comment 4

13 years ago
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.
Attachment #146642 - Flags: superreview?(dbaron)
Attachment #146642 - Flags: approval1.7?
Attachment #146642 - Flags: superreview?(dbaron) → superreview+

Comment 5

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

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #146642 - Flags: approval1.7? → approval1.7+
(Assignee)

Comment 6

13 years ago
I dont have cvs access, so if someone could check it in I'd appreciate ;)
Landed on the 1.7 branch.
Keywords: fixed1.7
Note, I landed a bustage fix to
embedding/tests/mfcembed/components/PrintProgressDialog.cpp on the branch as well.
(Assignee)

Comment 9

13 years ago
Created attachment 147446 [details] [diff] [review]
better fix for trunk
(Assignee)

Updated

13 years ago
Attachment #147446 - Flags: review?(caillon)
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?
(Assignee)

Comment 11

13 years ago
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 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.
Attachment #147446 - Flags: review?(caillon)
(Assignee)

Comment 13

13 years ago
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.
(Assignee)

Updated

13 years ago
Attachment #147446 - Attachment is obsolete: true
(Assignee)

Comment 14

13 years ago
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).
Attachment #147714 - Flags: review?(caillon)
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.
(Assignee)

Comment 16

13 years ago
Ok, ccing blizzard and bryner. Maybe they have comment on the nsComObsolete.h
killoff. Will ask sr to dbaron.
(Assignee)

Comment 17

13 years ago
Created attachment 148119 [details] [diff] [review]
use PR_BEGIN/END_EXTERN_C
Attachment #147714 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #148119 - Flags: superreview?(dbaron)
Attachment #148119 - Flags: review?(caillon)
(Assignee)

Updated

13 years ago
Attachment #147714 - Flags: review?(caillon)
(Assignee)

Comment 18

13 years ago
Created attachment 148120 [details] [diff] [review]
Readd the build bustage fix I lost somewhere
Attachment #148119 - Attachment is obsolete: true
(Assignee)

Updated

13 years ago
Attachment #148119 - Flags: superreview?(dbaron)
Attachment #148119 - Flags: review?(caillon)
(Assignee)

Updated

13 years ago
Attachment #148120 - Flags: superreview?(dbaron)
Attachment #148120 - Flags: review?(caillon)
Attachment #148120 - Flags: superreview?(dbaron) → superreview+
Attachment #148120 - Flags: review?(caillon) → review+
attachment 148120 [details] [diff] [review] checked into trunk 2004-05-18 11:09 PDT.

Marco, is anything left to be done here?
Assignee: general → marco
(Assignee)

Comment 20

13 years ago
No, I think this is fixed now. Thanks!
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 21

13 years ago
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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.