GNOME Bugzilla – Bug 401970
Numerous regressions in GtkPrint migration
Last modified: 2007-04-06 05:12:25 UTC
Having just committed the patch for bug #394182, which fixes all compiler warnings in GtkHtml as of version 3.13.5 [1], a subsequent build shows the following new warnings introduced in version 3.13.6: gtkhtml.c: In function 'gtk_html_print': gtkhtml.c:4171: warning: passing argument 2 of 'html_engine_print' from incompatible pointer type gtkhtml.c: In function 'gtk_html_print_with_header_footer': gtkhtml.c:4193: warning: passing argument 2 of 'html_engine_print_with_header_footer' from incompatible pointer type gtkhtml.c: In function 'gtk_html_print_get_pages_num': gtkhtml.c:5896: warning: passing argument 2 of 'html_engine_print_get_pages_num' from incompatible pointer type htmlengine-print.c: In function 'print_with_header_footer': htmlengine-print.c:186: warning: implicit declaration of function 'gtk_html_debug_dump_tree' htmlengine-print.c:163: warning: unused variable 'cr' htmlprinter.c: In function 'insure_config': htmlprinter.c:52: warning: passing argument 1 of 'gnome_print_job_get_config' from incompatible pointer type htmlprinter.c:52: warning: assignment from incompatible pointer type htmlprinter.c: In function 'html_printer_new': htmlprinter.c:777: warning: assignment from incompatible pointer type htmlprinter.c:779: warning: assignment from incompatible pointer type htmlprinter.c: In function 'get_tmargin': htmlprinter.c:107: warning: control reaches end of non-void function htmlprinter.c: At top level: htmlprinter.c:50: warning: 'insure_config' defined but not used testgtkhtml.c: In function 'print_preview_cb': testgtkhtml.c:330: warning: passing argument 6 of 'gtk_html_print_with_header_footer' from incompatible pointer type test.c: In function 'print_cb': test.c:170: warning: assignment from incompatible pointer type test.c:171: warning: assignment from incompatible pointer type test.c:173: warning: passing argument 1 of 'gnome_print_config_set' from incompatible pointer type test.c:176: warning: passing argument 1 of 'gnome_print_config_set' from incompatible pointer type test.c:178: warning: passing argument 2 of 'gtk_html_print' from incompatible pointer type Further inspection of those warnings yields some disturbing discoveries: - In the following functions, a GnomePrintContext pointer is cast to a GtkPrintContext pointer. Those types are incompatible, and therefore calling these public functions with the declared argument types will likely result in a segmentation fault: gtk_html_print() gtk_html_print_with_header_footer() gtk_html_print_get_pages_num() - In html_printer_new(), a GnomePrintJob pointer is cast to a GtkPrintOperation pointer. These types are incompatible, and therefore calling this function with the declared argument types will likely result in a segmentation fault. - The function get_bmargin() has a return type of gdouble but lacks a return statement. - The test programs testgtkhtml.c and test.c each cause a segmentation fault when attempting to print, due to more illegal mixing of GnomePrint and GtkPrint APIs in version 3.13.6. The first item is by far the most serious. The public API in GtkHtml 3.13.6 is broken. Those three functions either need to be updated with the correct parameter types or removed from GtkHtml before the next release. [1] Excluding warnings caused by bug #363033.
Created attachment 82170 [details] [review] Proposed patch Crikey, I never posted a patch for this! I was trying to get the test programs that test printing working again but haven't been able to yet. So this patch fixes everything reported above except for the test programs (src/test.c and src/testgtkhtml.c). I'll submit a second patch when I get them printing again. Other than warnings fixes there are two other items of interest: 1) I removed GtkHtml's dependency on libgnomeprint[ui]. 2) Minor API change. I removed gtk_html_print_set_master() since it no longer does anything useful, and takes a GnomePrintJob (which comes from libgnomeprint). The printing API is already in flux so I don't anticipate this being an issue. In 3.13.6, gtk_html_print_set_master() assigns the GnomePrintJob to the "print_master" field in GtkHTMLPrivate, where it eventually winds up in the "master" field of HTMLPrinter, which HTMLPrinter no longer uses. I ripped out all that internal linkage along with the public function.
Matthew, this looks OK. But I have some gtkhtml printer related stuff to be committed before this. We have to remove the libgnomeprint* dependecy. gtk_html_print() gtk_html_print_with_header_footer() gtk_html_print_get_pages_num() These 3 functions can be removed and the gtk_html_print_page functions should be renamed/used from here on. This was a ugly hack to get rid of API break then we got that we dont have to maintain that. So this has to be fixed. I have some patches towards that. I will do them over the weekend with Ebby and apply your patch accordingly and commit. Thanks for your great work as always.
Sounds good. Be sure to remove gtk_html_print_set_master() as well, unless your changes make it useful again. It's good to see the libgnomeprint* dependency get dropped. Now on to do the same with Evolution... :)
I guess I would remove that. Sure, lets start that next for evolution. It should be pretty straight forward.
Committed for 2.9.91
Reopening with an addendum. I got the test programs printing again.
Created attachment 83232 [details] [review] Proposed patch for test programs These make for good examples of how to use the new printing API.
Created attachment 83234 [details] [review] Revised patch for test programs Minor correction: pango_units_to_double() and pango_units_from_double() are too new to be using just yet.
Matthew, You ROCK!!!
Committed to Subversion trunk.
So, How is this API compatible with gtkhtml 3.8? Things like gnucash call gtk_html_print to print, and so won't build with 3.13.9x; if they're modified to work with 3.13.9x, then they won't work with 3.12.x and so on. I think this change probably warrants a rev to 3.9 (or 3.14?) of the gtkhtml API, so that programs like gnucash can continue to work.
The test program patch broke compilation in older build environments (see bug #412342). The solution is to substitute pango_language_get_default() with gtk_get_default_language() in testgtkhtml.c. The Pango function is too new to be using just yet.
Created attachment 83420 [details] [review] API bump patch Here's the (straightforward) patch I'm testing in the overlay for Gentoo. I'm also making the equivilent patch for evolution, but I haven't finished with the testing for that yet. I have verified that it co-habitates with gtkhtml-3.12.3 nicely.
Reopening once more for Daniel's and my patches (comment #12 and comment #13). Since this is becoming a catch-all for things that broke in the GtkPrint migration, I'm retitling it.
Wouldn't you bump the API to 3.10? It doesn't necessarily appear to match the package version.
Well, the last 4 API numbers were: 3 released with 3.0; 3.2, released with 3.2; 3.6 released with 3.6; and 3.8 released with 3.8. Following that progression, the next one should be 3.14 released with 3.14. I don't care if it's 3.9 or 3.10, tho.
Created attachment 83423 [details] [review] Patch to evo for new API version Here's the patch I'm using to test evo with the API bump patch above. Everything seems to work fine: both evo linked against the new API gtkhtml, and other things linked against the old API gtkhtml.
Dang, GTKHTML_CURRENT should be at 19. It cant be changed to zero. Ref: http://live.gnome.org/MaintainersCorner/Releasing Also I prefer to have the API version at 3.10 instead of 3.14.
I honestly have no opinion on 10 vs. 14, I did it soley based on the history of bumping. However, that page says: - If binary compatibility has been broken (eg removed or changed interfaces) # change to C+1:0:0 Thus, I believe it should be 0, not 19. (I originally had it 19, but changed it to 0...)
Created attachment 83475 [details] [review] Don't reset CURRENT After discussion with srag on IRC, we decided to not reset CURRENT. Here's a replacement patch with that change. The evo patch should work unchanged.
Please commit.
This has not been committed as yet and we are in Hard Code Freeze. Was your intention to have this in before the final release on the current dev series or the next one?
Sounds like it was intended to get into the current dev series.
Attachment (id=83234) appears to have been committed (not updated in bugzilla though) while the other two need to be committed. Is this to set the numbers correctly for a binary break already committed or does it just introduce it now ? I am not sure if i can just commit these patches while the Hard Code freeze is on. A request will have to come from the patch submitter/reviewer or the package maintainer - and yes, this is blocking the Evo release now owing to attachment (id=83423)
Update the status of attachment (id=83234).
Srini : Your comment #18 bites comment #21 as the approved patch does not set the API_VERSION to 3.10 as you have preferred but to 3.14 instead... - puzzled_harish
(In reply to comment #24) > Attachment (id=83234) appears to have been committed (not updated in bugzilla > though) while the other two need to be committed. My bad for not updating the attachment status. > Is this to set the numbers correctly for a binary break already committed or > does it just introduce it now ? I am not sure if i can just commit these > patches while the Hard Code freeze is on. A request will have to come from the > patch submitter/reviewer or the package maintainer - > and yes, this is blocking the Evo release now owing to attachment (id=83423) The API/ABI break occurred in 2.9.6 with the GtkPrint migration, but the soname was never properly bumped. Seems a bit late to be cleaning up the mess now.
The remaining patches appear to have been committed, and API version 3.14 was released in GNOME 2.18. Closing this bug.