GNOME Bugzilla – Bug 126456
Yelp should be able to print
Last modified: 2005-12-12 18:20:19 UTC
Yelp should work with gnome-print so you can print pages from a manual. Not even all of them at once, although that'd be cool, but at least the single page.
This would be sweet. Feature, punting.
*** Bug 142705 has been marked as a duplicate of this bug. ***
*** Bug 171781 has been marked as a duplicate of this bug. ***
*** Bug 311345 has been marked as a duplicate of this bug. ***
Created attachment 55222 [details] [review] Printing support for yelp Attached is a patch for printing in Yelp. It uses libgnomeprint*. Can people please test this (preferrably with an actual printer) and let me know how it goes so I can actually get this in before the next release (2.14). Cheers Don
Thanks for the patch! First, I noticed you copied some of the code from Epiphany but didn't add the copyright notices of the files the code came from to the files it ended up in; please fix that. I'm mostly going to review the gecko parts of the patch and some things which I noticed; the rest if for someone else. +++ src/yelp-gecko-services.h 22 Nov 2005 16:21:38 -0000 +#include "nsError.h" +#undef MOZILLA_INTERNAL_API +#include <nsEmbedString.h> +#define MOZILLA_INTERNAL_API 1 + +#include <nsIPrintProgressParams.h> Please move those includes to the .cpp file. +++ src/yelp-html.cpp 22 Nov 2005 16:21:40 -0000 [many includes added...] yelp_html_print(...) This code should be moved to Yelper (which will also simplify it, since you have mWebBrowser already available), and just called from here. + nsCOMPtr<nsIWebBrowserPrint> print(do_GetInterface(webBrowser)); + + if (print) { This will always succeed, write it like this in Yelper: nsCOMPtr<...> print (do_GetInterface (mWebBrowser, &rv)); NS_ENSURE_SUCCESS (rv, rv); + html_set_print_settings (info, settings); That function belongs in Yelper or maybe yelp-gecko-service instead. + nsIDOMWindow *cwin; + webBrowser->GetContentDOMWindow (&cwin); This is totally leaking :). After moving this to Yelper you can just use mDOMWindow. + print->PrintPreview (settings, cwin, NULL); s/NULL/nsnull/ (same, but it's more consistent). yelp_html_preview_navigate Same as above, move to Yelper. yelp_html_preview_end Same. +new_window_orphan_cb [...] [and more stuff with opening new windows] What's trying to open XUL windows here? I don't think we want that, we should prevent it instead of implementing popup windows. +NS_GENERIC_FACTORY_CONSTRUCTOR(GPrintingPromptService) + +static const nsModuleComponentInfo sAppComps[] = { + { + G_PRINTINGPROMPTSERVICE_CLASSNAME, + G_PRINTINGPROMPTSERVICE_CID, + G_PRINTINGPROMPTSERVICE_CONTRACTID, + GPrintingPromptServiceConstructor + }, +}; + +yelp_html_initialize [..] +html_setup_printing Move those to the gecko services file, or a new file for global gecko stuff. yelp-html is just for the embed widget implementation. + single = gtk_moz_embed_single_get (); + if (single == NULL) { + g_warning ("Failed to get singleton embed object!\n"); + } This should never fail; remove the check or assert. + if (sAppComps[0].mRegisterSelfProc) + { + rv = sAppComps[0].mRegisterSelfProc (cm, nsnull, nsnull, nsnull, &sAppComps[0]); + + if (NS_FAILED (rv)) + { + g_warning ("Failed to register-self for %s\n", sAppComps[0].mDescription); + ret = FALSE; + } + } Since your only component has no register func, you can remove this bit. +html_set_print_settings Different file too. +#ifndef HAVE_GECKO_1_9 You need to add the gecko version check stuff from epiphany's configure.ac to yelp's configure for this to work. +++ src/yelp-main.c 22 Nov 2005 16:21:41 -0000 gtk_moz_embed_set_comp_path (MOZILLA_HOME); - + yelp_html_initialize (); You could move the set_comp_path to the rest of the gecko init with set_profile_path and the component registration. +++ src/yelp-print.c 22 Nov 2005 16:21:43 -0000 +yelp_print_info_free + if (info->header_left_string) + g_free (info->header_left_string); [many more like this] g_free is NULL-safe. +++ src/yelp-window.c 22 Nov 2005 16:21:48 -0000 + action = gtk_action_group_get_action (priv->action_group, + "PrintDocument"); + if (action) + g_object_set (G_OBJECT (action), "sensitive", FALSE, NULL); Action will always be non-NULL here, no need for the check. (Same for more instances of this) +window_print_page_cb + result = gnome_vfs_open (&handle, uri, GNOME_VFS_OPEN_READ); Check the result code for success.
Created attachment 55256 [details] [review] Updated patch Thanks for reviewing the patch! From the comment: >First, I noticed you copied some of the code from Epiphany but didn't add the >copyright notices of the files the code came from to the files it ended up in; >please fix that. Sorry. Thought I had added them. Should be fixed in this patch. I've made all the fixes requested (I think) I've moved all the html_print type functions to Yelper and made the setting of print options a private member of Yelper as well (is this the best way of doing this?). I have no idea where the XUL window stuff came from. I thought it was necessary for doing printing, but removing it doesn't do any harm, so its all gone now :) Moved the html_setup_printing to yelp-gecko-services under the name yelp_register_printing. >+#ifndef HAVE_GECKO_1_9 >You need to add the gecko version check stuff from epiphany's configure.ac to >yelp's configure for this to work. Is this covered by the attachment for bug #322171? If so, I'll add a dependancy to this bug. If not, I'll look into the configure stuff a bit more.
The configure bits are covered by bug 322171, yes.
+ nsCOMPtr<nsIPrintSettings> settings; + + print->GetGlobalPrintSettings (getter_AddRefs (settings)); rv = print->Get... and NS_ENSURE_SUCCESS (rv, rv); + if (!preview) + print->Print (nsnull, listener); + else { + print->PrintPreview (nsnull, mDOMWindow, nsnull); + print->GetPrintPreviewNumPages (prev_pages); rv = print->Print rv = print->PrintPreview rv |= print->GetPrint... + print->PrintPreviewNavigate (0, page_no); + + return rv; return print->Print... is simpler :) + print->ExitPrintPreview (); + + return rv; Same. Index: src/yelp-gecko-services.cpp +#include "nsError.h" +#undef MOZILLA_INTERNAL_API +#include <nsEmbedString.h> +#define MOZILLA_INTERNAL_API 1 + +#include "mozilla-config.h" +#include "config.h" mozilla-config.h and config.h need to be included before all other headers. Index: src/yelp-gecko-services.h +#define G_PRINTINGPROMPTSERVICE_CID \ +{ /* 5998a2d3-88ea-4c52-b4bb-4e7abd0d35e0 */ \ Use a different CID (uuidgen command), I generated one for you: #define G_PRINTINGPROMPTSERVICE_CID \ { /* dbf438d3-5f62-4978-a700-6fc39447477c */ \ 0xdbf438d3, 0x5f62, 0x4978, \ { 0xa7, 0x00, 0x6f, 0xc3, 0x94, 0x47, 0x47, 0x7c } } Index: src/yelp-html.cpp: + gtk_moz_embed_set_profile_path (profile_path, MOZILLA_PROFILE_NAME); It just occurred to me that now that yelp uses a mozilla profile, we should probably set some additional prefs to prevent unnecessary data storage, like history, i.e. bug 322170.
On further thought, why is the mozilla profile needed, actually?
Created attachment 55377 [details] [review] Patch Updated patch for comments above plus a couple of other slightly stupid things I missed. Mozilla profile is in due to bug #156136 (and others).
Hmm, bug 156136 was just the set_comp_path setting, not the profile path. Afaics, nothing in that bug indicates a need for a profile.
Created attachment 55426 [details] [review] Once more Sigh. I'll get there eventually. The profile stuff is from the original printing patch that used the mozilla printing dialog. This removes the profile bits but keeps the html_initialize function which, at the moment, just sets the comp path.
This patch has now been committed to yelp and is part of yelp 2.13.2 (in an effort to get a bit more testing on it). Any printing bugs should be filed as new bug reports. Closing. Thanks