Skip to content

Fix 6684. Add call to gimp_foo_close_popup.

Lloyd Konneker requested to merge bootchk/gimp:6684select_dialog_not_close into master

Disclaimer:

I don't deeply understand the problem or the fix. The code involves events, callbacks, and the wire; it's hard for me to follow by just reading the code. To understand the code, I harnessed the code with g_printerr.

What broke it:

I think commit 6e80a232 is the likely breakage. Commit 6e80a232 by mitch, one year ago, for the new plugin API.

About the fix:

The fix is in files libgimp/gimpfooselect.c where foo is in [brush, palette, pattern, font, gradient].

Commit 6e80a232 moved a call to gimp_brush_close_popup (brush_callback), from its original location in the function gimp_brush_select_destroy() to a function gimp_brush_data_free(), which is after the brush_callback is removed. The brush_callback is the name of the temporary procedure. You can't remove the temporary procedure first because the code that closes the popup uses the name of the temporary procedure to find the gtk_dialog to close. (See app/gui/gui-vtable.c line 784)

The fix adds the call to gimp_brush_close_popup() to its original location. The fix leaves the same call in the function gimp_brush_data_free, where commit 6e80a232 moved it. That is, there are now two calls to gimp_brush_close_popup(). As far as I know, it is OK to leave the second call to gimp_brush_close_popup, but maybe it is now unnecessary.

My fix is largely intuition: when creating the BrushSelect in gimp_brush_select_new(), the order is:

  • create temporary procedure "brush_callback"
  • create the popup dialog, passing the "brush_callback"

so that when destroying the BrushSelect, you must reverse the order:

  • close the popup (since it knows the temporary procedure callback)
  • remove the temporary procedure

Which AFAIK is NOT the order used by commit 6e80a232. This MR just restores the original order of destruction.

Testing:

I tested using Filters>Development>ScriptFu>Test>Sphere. Click on five buttons to open five select dialogs. Then click on OK to close the main dialog. All five select dialogs should close also.

I tested on both AM build with ASAN, and meson build.

Merge request reports