367907 - allow printing to PDF file on gtk2-cairo

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

(Not sure this is the correct component; maybe -> Widget::Gtk ?)

Printing to file always prints to a PS file. That's because sDeviceContextSpecGTK::GetSurfaceForPrinter always creates a PS surface:

442 #ifdef USE_PDF
443   gfxPDFSurface *surface = new gfxPDFSurface(path, gfxSize(w, h));
444 #else
445   gfxPSSurface *surface = new gfxPSSurface(path, gfxSize(w, h));
446 #endif

and USE_PDF is undefined.

In Epiphany, I want to offer the choice to print to a PS or PDF file; so I need a programmatic switch here, not a compile time option.

Attached patch implements this by adding a new attribute "outputFormat" to nsIPrintSettings with values kOutputFormatPS|PDF.

Comment 1

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: patch

Comment 2

[Stuart Parmenter]

Comment on attachment 252484 [details] [diff] [review]
patch

Should we have an OutputFormatNative or something that this defaults to?  Setting this to PS or PDF on windows and mac won't do what you'd hope for with this... thoughts?

Comment 3

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: add kOutputFormatNative and default to it

Yes, I think it makes sense to have kOutputFormatNative as default.

Comment 4

[Stuart Parmenter]

Comment on attachment 252764 [details] [diff] [review]
add kOutputFormatNative and default to it

actually, is there any reason to have mOutputFormat on the devicecontextspec?  would it be easier to just call mPrintSettings->GetOutputFormat in GetSurfaceForPrinter?

Comment 5

[Christian Persch (GNOME) (away; not receiving bug mail)]

(In reply to comment #4)
> (From update of attachment 252764 [details] [diff] [review])
> actually, is there any reason to have mOutputFormat on the devicecontextspec? 
> would it be easier to just call mPrintSettings->GetOutputFormat in
> GetSurfaceForPrinter?

I just followed how the other print settings are cached in the devicecontextspec; if you prefer it without that, I change the patch.

Comment 6

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch

Updated patch to trunk. I removed the caching of the setting, and added a null check.

Comment 7

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: unbitrotted, updated patch

Comment 8

[Christian Persch (GNOME) (away; not receiving bug mail)]

Does this need sr, or is this ready for checkin?

Comment 9

[Steffen Wilberg]

I guess it does need sr (see the log: http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/widget/src/gtk2/nsDeviceContextSpecG.cpp), but you can request that from pavlov as well. Or ask roc.

Comment 10

[Steffen Wilberg]

You now need to request approval1.9 as well, and provide a short evaluation of risk and benefit of the patch...
http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/569a23ce266a451/265344ae5291dee0

Comment 11

[Christian Persch (GNOME) (away; not receiving bug mail)]

The patch has no risk at all since it's just replacing an |#ifdef| with an |if ()| which will take the same branch as the #ifdef by default (the default value is kOutputFormatNative; and no code in mozilla sets it to any other value). The benefit is for embedders (Epiphany) that want to allow printing to PDF file instead of PS file.

Comment 12

[Peter Weilbacher]

Patch checked into trunk.