After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 482089 - GtkPrintOperation check for null default signal handler breaks gtkmm
GtkPrintOperation check for null default signal handler breaks gtkmm
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Printing
2.12.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2007-10-01 08:19 UTC by Murray Cumming
Modified: 2007-10-05 14:18 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Murray Cumming 2007-10-01 08:19:37 UTC
This change added a check for whether GtkPrintOperationClass:paginate is NULL. But language bindings (such as gtkmm) provide their own implementations of default signal handlers (to make it easier to re-implement them). So, for gtkmm, paginate is never NULL. This apparently stops gtkmm applications from printing: See bug #473923.

--- gtk/gtkprintoperation.c     (revision 17698)
+++ gtk/gtkprintoperation.c     (revision 17703)
@@ -2063,7 +2063,8 @@
          goto out;
        }

-      if (g_signal_has_handler_pending (data->op, signals[PAGINATE], 0,
FALSE))
+      if (GTK_PRINT_OPERATION_GET_CLASS (data->op)->paginate != NULL ||
+          g_signal_has_handler_pending (data->op, signals[PAGINATE], 0,
FALSE))
        {
          gboolean paginated = FALSE;

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 17698)
+++ ChangeLog   (revision 17703)
@@ -2,6 +2,14 @@

        Merge from trunk:

+       * gtk/gtkprintoperation.c (print_pages_idle): Also check
+       the default handler when deciding whether to emit the
+       paginate signal.  (#345345, Yevgen Muntyan)
Comment 1 Tim Janik 2007-10-01 11:04:09 UTC
this check is total crack and should be removed alltogether.
for everyone interested, gsignal.c already does this for signal emissions, see gsignal.c:signal_check_skip_emission. murray, can you please simply remove the has_handler_pending bogosity?
Comment 2 Murray Cumming 2007-10-01 11:07:52 UTC
Great. I hope it's that simple.

I'd rather wait a bit for Mathias Clasen to comment. That seems polite, considering that he committed it.
Comment 3 Yevgen Muntyan 2007-10-01 11:29:12 UTC
IIRC, the check may not be simply removed, signal emission will set return value to default FALSE which is wrong in that context (I don't mean check shouldn't be removed at all, just saying it's not as simple as select-and-delete, IIRC. I may very well be wrong too).
Comment 4 Murray Cumming 2007-10-01 11:32:28 UTC
I guess that the correct way to specify a default return value for a signal is to implement a default signal handler.
Comment 5 Tim Janik 2007-10-01 11:51:14 UTC
(In reply to comment #4)
> I guess that the correct way to specify a default return value for a signal is
> to implement a default signal handler.

g_signal_emitv() can be used to specify a return value that'll be unaltered if no handler executed. for the g_signal_emit() call at hand though, the GSignal return value should default to FALSE if no handler is called.
Comment 6 Murray Cumming 2007-10-05 08:22:10 UTC
I have reverted the change in GTK+. I'll leave it to bug #473923 to provide a corrected fix for its specific problem.
Comment 7 Murray Cumming 2007-10-05 08:22:52 UTC
Sorry, I mean I'll leave it to #345345.
Comment 8 Yevgen Muntyan 2007-10-05 11:51:00 UTC
The check in question is weird and ugly, but it's not wrong. And here's what happens:

1) gtk has a bug: it totally ignores class hanlder.
2) gtkmm has a bug: it installs bogus class handler which returns a wrong value. Due to the gtk bug this gtkmm bug wasn't exposed.

Possible solutions:

1) Fix gtk and gtkmm. Not plausible, as this bug entry demonstrates.
2) Document that the class signal handler won't work, and ignore it.
Comment 9 Murray Cumming 2007-10-05 12:04:42 UTC
Please do this in the original #345345, so we don't have two bugs open for the same thing. In that bug, I request a test case for your original problem.
Comment 10 Yevgen Muntyan 2007-10-05 14:02:48 UTC
The original #345345 was fixed (and now re-created again, if I understand the comment #6 correctly). The comment about re-creating that bug is here, so I comment here.

If that bug isn't going to be fixed, these two are closed and everybody is happy. If that bug is going to be fixed (namely, if the default signal handler is taken into account, and return value of FALSE means "keep emitting paginate" as it happens with callbacks), then you got this #482089.
Comment 11 Murray Cumming 2007-10-05 14:18:47 UTC
#345345 has been reopened, because its patch was reverted. Again, please discuss that bug there.