Closed
Bug 241145
Opened 20 years ago
Closed 20 years ago
nsIPrintSettings should not include nsFont.h
Categories
(Core Graveyard :: GFX, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mpgritti, Assigned: mpgritti)
Details
(Keywords: fixed1.7)
Attachments
(2 files, 3 obsolete files)
995 bytes,
patch
|
caillon
:
review+
dbaron
:
superreview+
asa
:
approval1.7+
|
Details | Diff | Splinter Review |
54.55 KB,
patch
|
caillon
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
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•20 years ago
|
||
Assignee | ||
Comment 2•20 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 3•20 years ago
|
||
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•20 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•20 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•20 years ago
|
||
I dont have cvs access, so if someone could check it in I'd appreciate ;)
Comment 8•20 years ago
|
||
Note, I landed a bustage fix to embedding/tests/mfcembed/components/PrintProgressDialog.cpp on the branch as well.
Assignee | ||
Comment 9•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #147446 -
Flags: review?(caillon)
Comment 10•20 years ago
|
||
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•20 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 12•20 years ago
|
||
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•20 years ago
|
||
I'll see if I can setup a windows build here and get someone to test on Mac.
Assignee | ||
Updated•20 years ago
|
Attachment #147446 -
Attachment is obsolete: true
Assignee | ||
Comment 14•20 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 15•20 years ago
|
||
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•20 years ago
|
||
Ok, ccing blizzard and bryner. Maybe they have comment on the nsComObsolete.h killoff. Will ask sr to dbaron.
Assignee | ||
Comment 17•20 years ago
|
||
Attachment #147714 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #148119 -
Flags: superreview?(dbaron)
Attachment #148119 -
Flags: review?(caillon)
Assignee | ||
Updated•20 years ago
|
Attachment #147714 -
Flags: review?(caillon)
Assignee | ||
Comment 18•20 years ago
|
||
Attachment #148119 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #148119 -
Flags: superreview?(dbaron)
Attachment #148119 -
Flags: review?(caillon)
Assignee | ||
Updated•20 years ago
|
Attachment #148120 -
Flags: superreview?(dbaron)
Attachment #148120 -
Flags: review?(caillon)
Attachment #148120 -
Flags: superreview?(dbaron) → superreview+
Updated•20 years ago
|
Attachment #148120 -
Flags: review?(caillon) → review+
Comment 19•20 years ago
|
||
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•20 years ago
|
||
No, I think this is fixed now. Thanks!
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 21•20 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.
Updated•15 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•