Closed
Bug 367829
Opened 18 years ago
Closed 17 years ago
not possible to build w/o MOZ_ENABLE_POSTSCRIPT
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
WONTFIX
mozilla1.9alpha8
People
(Reporter: wolfiR, Assigned: db48x)
References
Details
Attachments
(1 file, 5 obsolete files)
1.66 KB,
patch
|
pavlov
:
review-
|
Details | Diff | Splinter Review |
(don't know if this is widget or Printing) I used to build w/o MOZ_ENABLE_POSTSCRIPT on trunk which is not possible anymore since bug 323928 landed. This attachment from bug 323928 doesn't fix the build https://bugzilla.mozilla.org/attachment.cgi?id=247531 : nsDeviceContextSpecG.o: In function `nsDeviceContextSpecGTK::GetSurfaceForPrinter(gfxASurface**)':nsDeviceContextSpecG.cpp:(.text+0x607): undefined reference to `nsPrintJobFactoryGTK::CreatePrintJob(nsDeviceContextSpecGTK*, nsIPrintJobGTK*&)'
Comment 2•17 years ago
|
||
Attachment #260381 -
Flags: review?
Updated•17 years ago
|
Attachment #260381 -
Flags: review? → review?(roc)
Comment 3•17 years ago
|
||
also ifdef-ed nsPrintJobFactoryGTK::CreatePrintJob
Attachment #260381 -
Attachment is obsolete: true
Attachment #260382 -
Flags: review?(roc)
Attachment #260381 -
Flags: review?(roc)
Attachment #260382 -
Flags: review?(roc) → review+
Comment 4•17 years ago
|
||
It should help for http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTest Xulrunner Min
Keywords: checkin-needed
Updated•17 years ago
|
Attachment #260382 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #260382 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Assignee: nobody → romaxa
Comment 5•17 years ago
|
||
This patch doesn't apply cleanly against current trunk, can you update it?
Keywords: checkin-needed
Comment 6•17 years ago
|
||
whole function ifdefed, return NS_ENSURE_SUCCESS to see problem in debug builds..
Attachment #260382 -
Attachment is obsolete: true
Updated•17 years ago
|
Keywords: checkin-needed
Comment 8•17 years ago
|
||
That's not "updated to trunk" - it's a different patch. If you want to do that instead, please get it re-reviewed and re-approved, or if you think it's the same thing, please persuade your reviewer to check it in, because to the random peons pulled in by checkin-needed, that's too much change.
Keywords: checkin-needed
Comment 9•17 years ago
|
||
Attachment #276404 -
Attachment is obsolete: true
Comment 10•17 years ago
|
||
I hope this is more looks like updated to trunk?
Updated•17 years ago
|
Keywords: checkin-needed
Comment 11•17 years ago
|
||
Comment on attachment 276404 [details] [diff] [review] Update to trunk for checkin > NS_IMETHODIMP nsDeviceContextSpecGTK::GetSurfaceForPrinter(gfxASurface **aSurface) > { >+#ifndef MOZ_ENABLE_POSTSCRIPT >+ NS_ENSURE_SUCCESS(NS_ERROR_FAILURE, NS_ERROR_FAILURE); >+#else It's not my module, but to me success(failure) is needlessly opaque. Please spell it out NS_NOTREACHED("Shouldn't call this if no Postscript support"); return NS_ERROR_FAILURE; Or NS_WARNING() if you just want a debug message without the assert. NS_NOTYETIMPLEMENTED() if you think you'll flesh this out some day.
Attachment #276404 -
Attachment is obsolete: false
Comment 12•17 years ago
|
||
Checked in attachment 278004 [details] [diff] [review]. Please close the bug if needed. mozilla/widget/src/gtk2/Makefile.in 1.67 mozilla/widget/src/gtk2/nsDeviceContextSpecG.cpp 1.91
Updated•17 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 13•17 years ago
|
||
Comment on attachment 278004 [details] [diff] [review] Update to trunk2 for checkin + nsresult rv = NS_ERROR_FAILURE; +#ifndef MOZ_ENABLE_POSTSCRIPT + nsPrintJobFactoryGTK::CreatePrintJob(this, mPrintJob); +#endif if (NS_FAILED(rv)) return rv; Isn't this missing an "rv =" inside the ifndef?
Comment 14•17 years ago
|
||
Fixed that: mozilla/widget/src/gtk2/nsDeviceContextSpecG.cpp 1.92
Reporter | ||
Comment 15•17 years ago
|
||
Comment on attachment 278004 [details] [diff] [review] Update to trunk2 for checkin >Index: src/gtk2/nsDeviceContextSpecG.cpp >=================================================================== > >- nsresult rv = nsPrintJobFactoryGTK::CreatePrintJob(this, mPrintJob); >+ nsresult rv = NS_ERROR_FAILURE; >+#ifndef MOZ_ENABLE_POSTSCRIPT >+ nsPrintJobFactoryGTK::CreatePrintJob(this, mPrintJob); >+#endif > if (NS_FAILED(rv)) > return rv; Hmm? Why did that turn into an #ifndef? I would expect that we want to create a print job if POSTSCRIPT is actually enabled?
Comment 16•17 years ago
|
||
Sigh, fixed in rev 1.93.
Comment 17•17 years ago
|
||
sorry, i didn't look closely enough at this patch. We should be getting rid of MOZ_ENABLE_POSTSCRIPT #ifdefs not adding new ones. we should remove the --enable/disable-postscript option from configure entirely.
Comment 18•17 years ago
|
||
i've filed 394413 on removing MOZ_ENABLE_POSTSCRIPT entirely.
Comment 19•17 years ago
|
||
This patch caused bug 394407: MOZ_ENABLE_POSTSCRIPT seems to not be defined when building widget/src/gtk2/nsDeviceContextSpecG.cpp. Or something. The .i file has: nsresult rv = ((nsresult) 0x80004005L); if (((__builtin_expect(!!((rv) & 0x80000000), 0)))) return rv; which is clearly bad.
Comment 20•17 years ago
|
||
I'm going to back this patch out this weekend, per stuart's comment and because this caused regressions. If anyone beats me to it, great.
Comment 21•17 years ago
|
||
Note that the patch in bug 394160 switches to the right define. Pending that landing, I have backed this out to fix smoketest blocker bug 394407.
Assignee | ||
Comment 22•17 years ago
|
||
Ok, this fix seems to have been lost in the shuffle. It caused a regression, and bug 394160 had a patch that fixed the regression. This bug was then backed out, and marked as fixed because the new patch would leave this bug in a fixed state. However, the new bug was then closed because it was just a regression from a patch that got backed out, and thus was no longer needed. I'll attach a patch that corrects romaxa's typo for review.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 23•17 years ago
|
||
Assignee: romaxa → db48x
Attachment #276404 -
Attachment is obsolete: true
Attachment #278004 -
Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #282927 -
Flags: review?
Assignee | ||
Updated•17 years ago
|
Attachment #282927 -
Flags: review? → review?(roc)
Comment 24•17 years ago
|
||
I believe Stuart said that this bug as filed should be wontfixed.
Comment 25•17 years ago
|
||
>Created an attachment (id=282927) [details] >367829-5.dif This is wrong, MOZ_ENABLE_POSTSCRIPT cannot be used there, USE_POSTSCRIPT is ok. It makes printing broken by default... Bug 394160. Also there still one using not initialized mPrintJob without check in end document... https://bugzilla.mozilla.org/show_bug.cgi?id=394160 https://bugzilla.mozilla.org/attachment.cgi?id=278825
Comment 26•17 years ago
|
||
Merge https://bugzilla.mozilla.org/attachment.cgi?id=282927 + https://bugzilla.mozilla.org/attachment.cgi?id=278825 = Probably good result.
Assignee | ||
Comment 27•17 years ago
|
||
Yes, it would be nice to get bug 394413 done, but it's not even assigned to anybody. I don't know anything about how cairo or gfx work, so in the mean time this bustage fix seems like a good way to go. It won't add much to the work required to fix 394413.
Assignee | ||
Comment 28•17 years ago
|
||
thanks romaxa, I had forgotten about the other change
Attachment #282927 -
Attachment is obsolete: true
Attachment #283040 -
Flags: review?(roc)
Attachment #282927 -
Flags: review?(roc)
Comment 29•17 years ago
|
||
>widget/src/gtk2/Makefile.in ..... >+ifdef USE_POSTSCRIPT I guess USE_POSTSCRIPT is not defined in Make... there are MOZ_.... USE_POSTSCRIPT defined here: as -D... http://mxr.mozilla.org/seamonkey/source/widget/src/gtk2/Makefile.in#157 If we want to use USE_POSTSCRIPT in Makefile, than we have to define it somewhere...
Comment 30•17 years ago
|
||
as i said before, we should just remove all of these ifdefs.
Updated•17 years ago
|
Attachment #283040 -
Flags: review?(roc) → review-
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•