GNOME Bugzilla – Bug 672694
Set correct paper size for new printer
Last modified: 2012-04-13 12:18:44 UTC
Created attachment 210429 [details] [review] patch setting paper size correctly Current gnome-control-center doesn't set correct paper size according to locale for new printers (it always sets Letter). With new cups-pk-helper-0.2.2 it is possible to set it correctly using new method PrinterAddOption (by setting option "media" to "iso-a4" or "na-letter"). Attached patch uses this new method and sets correct paper size. Marek
Created attachment 210430 [details] [review] the same patch with ignore spaces turned on
Review of attachment 210429 [details] [review]: ::: panels/printers/pp-new-printer-dialog.c @@ +931,3 @@ if (error->domain == G_DBUS_ERROR && + (error->code == G_DBUS_ERROR_SERVICE_UNKNOWN || + error->code == G_DBUS_ERROR_UNKNOWN_METHOD)) I don't really like this function at all. It should look at introspection data, not call the method with the wrong arguments and see if it fails. granted, this commit didn't add the function, so I don't think it makes sense to rewrite it in this commit. @@ +935,3 @@ else result = TRUE; + g_error_free (error); ah another fix. I feel like the dbus_method_available fixes should be in separate commits. @@ +1802,3 @@ + value = g_strdup ("iso-a4"); + else + paper_size = "A4"; I don't really like how value and paper size are used for subtly different purposes in the same function. maybe it would be better to do two different helper functions for the two different possible dbus calls? @@ -1830,3 @@ - pp->devices[device_id].display_name, - "PageSize", - &array_builder), so we were already passing a page size in before? Was it getting ignored? It only worked if the ppd file was right? What's the point of this old call?
(In reply to comment #3) > Review of attachment 210429 [details] [review]: > @@ +935,3 @@ > else > result = TRUE; > + g_error_free (error); > > ah another fix. I feel like the dbus_method_available fixes should be in > separate commits. OK, I'll remove it. > @@ +1802,3 @@ > + value = g_strdup ("iso-a4"); > + else > + paper_size = "A4"; > > I don't really like how value and paper size are used for subtly different > purposes in the same function. maybe it would be better to do two different > helper functions for the two different possible dbus calls? I'm going to remove calling of the PrinterAddOptionDefault method. > @@ -1830,3 @@ > - > pp->devices[device_id].display_name, > - > "PageSize", > - > &array_builder), > > so we were already passing a page size in before? Was it getting ignored? It > only worked if the ppd file was right? What's the point of this old call? It was set at another place so the application could get into situation (and usually did) that it didn't see the change. Marek
You can see https://bugzilla.redhat.com/show_bug.cgi?id=749270 and https://bugzilla.redhat.com/show_bug.cgi?id=749235 for more informations.
Created attachment 210612 [details] [review] patch setting media size correctly
Created attachment 210613 [details] [review] the same patch with ignore spaces turned on
Review of attachment 210613 [details] [review]: ::: panels/printers/pp-new-printer-dialog.c @@ +1779,3 @@ + /* Set default media size according to the locale */ + if (g_str_equal (gtk_paper_size_get_default (), GTK_PAPER_NAME_LETTER)) Split this out in a separate function please.
Comment on attachment 210613 [details] [review] the same patch with ignore spaces turned on I've committed the patch with the change you wanted. Regards Marek