GNOME Bugzilla – Bug 613973
Menu popup() protect against die
Last modified: 2010-04-25 15:15:29 UTC
Created attachment 157125 [details] [review] G_EVAL around the positioning func for Menu popup() I wonder if $menu->popup() could have an eval to protect against die() in the positioning function. In some of my code an error caused a persistent X grab, locking the screen until the client program is killed.
Review of attachment 157125 [details] [review]: I heartily condone this change. My one reservation was about eating errors, but you've addressed that with the g_warning(). Now, if you have a g_log handler installed, and it decides to die on that warning, you have the same issue, right?
Is it permitted to die in a log handler? There'd have to be quite a few places that'd be a bad thing wouldn't there? The gperl_log_func() callback invocation may even be another of the various GPerlCallbacks which would benefit from eval protection. For the popup I'm open to ideas for the right log level and domain. "Error" may be better than warning, but non-fatal, still continuing with the code, not jumping out.
Review of attachment 157125 [details] [review]: Looks good to me in general. Two minor things: ::: t/GtkMenu.t @@ +142,3 @@ + 'popup positioning die() is not fatal'); +} + What's with $popup_runs here? Looks like dead code. ::: xs/GtkMenu.xs @@ +88,1 @@ Here and in the previous chunk above, you use spaces for indentation, but we use tabs in XS code.
Created attachment 157782 [details] [review] second effort G_EVAL popup() Err, umm, that looks a bit doubtful on re-reading. I think $popup_runs was to allow it to skip if the menu doesn't pop up. Is the the reason for $i_know_you? At any rate second effort attached. (I've lately tried to get the tabs mode right from my .emacs, but I'm also forever switching between my own xs-mode based on c-mode, and plain text-mode or pod-mode the xs directives upset the C parsing ...)
Committed. Thanks.