GNOME Bugzilla – Bug 340690
delete_ping_timeout_func() does not pass timestamp first as metacity-dialog.c expects
Last modified: 2006-08-21 19:43:09 UTC
Title pretty much says it all. We should probably use GOption or something in metacity-dialog.c to get rid of the crufty parsing and hardcoding of positions of parameters. Ought to be a good task for someone new.
Created attachment 67912 [details] [review] Use GOption Use GOption to parse args.
Firstly, all three options work in the new version generally as they did in the old. So that's good. Secondly-- a minor point-- it responds to "--help" now, and it might be nice if that told you it wasn't a program that users were expected to talk to. Thirdly, what is "--screen"? It wasn't in the original, and it does nothing in the new version. Fourthly, "--warn-about-no-sm-support" used to give an error if you gave it no arguments; it now segfaults. (Also, and not as part of this bug, we should heed the comments above copy_of_gdk_x11_window_set_user_time, since we're now requiring a new enough copy of GTK that we can do away with the call to XChangeProperty.)
Not having --screen in the original sounds like it was probably a bug (one which will still exist in the new version if it's not used somewhere); not passing the screen likely means that in multi-screen non-xinerama setups that the dialog will appear on the wrong screen in some cases. delete_ping_timeout_func() definitely passes the screen as an argument, so it seems that metacity-dialog likely may have honored it at some point? Marking as needs-work as per Thomas' other comments.
Created attachment 70502 [details] [review] Use GOption v2 Updated patch. "--warn-about-no-sm-support" no longer segfaults when given no args. "--help" now warns that the program should only be used by metacity. metacity-dialog gets called from three places: - delete_ping_timeout_func() - error_on_generic_command() - warn_about_lame_clients_and_finish_interact() The first two pass the --screen along. warn_about_lame_clients_and_finish_interact() probably should too and metacity-dialog should handle it. Should we file a new bug for this or try to fix it in this one? For now I have just focused on parsing the existing args with GOption. From the changelog: 2003-09-29 Havoc Pennington <hp@redhat.com> . . . Fix #103575, spawn child processes on proper screen. * src/keybindings.c (error_on_command): pass --screen to metacity-dialog (handle_run_command): launch user command with DISPLAY reflecting the screen you launch it from * src/delete.c (delete_ping_timeout_func): pass --screen to metacity-dialog And from #103575 comment #2: "This may also be needed for metacity-dialog etc. not sure we're handling that."
> Should we file a new bug for this or try to fix it in this one? For now > I have just focused on parsing the existing args with GOption. Let's file a separate one and just make this one about the GOption bits. :)
Committed, thanks Thomas!
(I filed bug 352293 about copy_of_gdk_x11_window_set_user_time() and bug 352298 about the lack of --screen handling in metacity-dialog)