Tue 14 Apr 2015 09:41:09 PM UTC, comment #16:
I pushed the patch here:
http://hg.savannah.gnu.org/hgweb/octave/rev/69f92e0affd2
Closing report.
|
Tue 14 Apr 2015 02:01:38 AM UTC, comment #15:
Go ahead and push it. The ui* series of functions need a lot of work and it would be nice if we could update them with bug fixes, rather than have to wait until 4.2 because we don't want to break ABI compatibility.
|
Sun 12 Apr 2015 09:43:06 AM UTC, comment #14:
So should I push it now or wait for 4.2? I am pretty sure it's innocuous for everything else than uicontextmenu related operations.
|
Sat 11 Apr 2015 05:11:17 PM UTC, comment #13:
Good point. We don't want to break ABI compatibility.
|
Sat 11 Apr 2015 04:05:01 PM UTC, comment #12:
Well, I am not sure this patch is a good candidate for minor release as it adds a public method to the uicontextmenu class. This will break ABI compatibility and AFAIU it's not good practice for minor releases.
|
Sat 11 Apr 2015 03:58:31 PM UTC, comment #11:
@Panxto: Maybe hold off until after the 4.0.0 release. We're anticipating that there will be a very quick 4.0.1 release 6-8 weeks after 4.0.0 to deal with whatever is uncovered by wide scale testing. Maybe it can go in then.
|
Sat 11 Apr 2015 02:54:23 PM UTC, comment #10:
@Rik: I think the patch is armless but as you are in the process of releasing 4.0 it's up to you wether I should push it now? If so can I add documentation for "uicontextmenu" property or did you already start grammar/spell-check task?
|
Sat 11 Apr 2015 02:47:22 PM UTC, comment #9:
Also works for me.
|
Sat 11 Apr 2015 02:29:18 PM UTC, comment #8:
Tested as well, works as expected. Thanks!
|
Sat 11 Apr 2015 01:52:01 PM UTC, comment #7:
I have attached a more general fix which includes BIST tests:
- setting "uicontextmenu" to [] only resets the value
- deleting a uicontextmenu object will also reset the "uicontextmenu" property of all the objects that have it as current.
(file #33639)
|
Sat 11 Apr 2015 12:35:05 PM UTC, comment #6:
It works for me - many thanks @pantxo!
|
Sat 11 Apr 2015 11:26:24 AM UTC, comment #5:
I have attached a simple patch that fixes the issue for me. However it does only handle the case of deleting a uicontextmenu and reseting its parent "uicontextmenu" property.
I am working on a more general solution that uses Rik's suggestion to do the cleanup in the uicontextmenu destructor. uicontextmenu objects will store a list of objects that may depend on them, and reset their "uicontextmenu" property at destruction if needed.
(file #33637)
|
Sat 11 Apr 2015 11:12:42 AM UTC, comment #4:
Thanks for looking into this - I was especially not aware of the usage of NaN in this particular setting.
I was also at first thinking that "set(F,'uicontextmenu',[])" was not a thing to do but, actually, this is and should be valid and has a useful role if you don't want to delete a uicontextmenu but just remove it temporarily and reuse it later on:
Unfortunately NaN works with Octave and not MATLAB, while [] works with MATLAB and not Octave.
Also flagging that there might be a link between this discussion and bug #44763.
|
Sat 11 Apr 2015 07:49:18 AM UTC, comment #3:
I am working on a simple solution that doesn't involve making use of the deletefcn (which we should keep available for the user).
|
Fri 10 Apr 2015 11:02:37 PM UTC, comment #2:
It appears that Matlab is also not that good about cleaning up after uicontextmenu's and requires the user to do it themselves (http://undocumentedmatlab.com/blog/uicontextmenu-performance).
|
Fri 10 Apr 2015 10:33:12 PM UTC, comment #1:
The problem appears to be that the C++ graphics code assumes that whenever an object is deleted it should be removed from it's parent's "children" property. In this case, the graphics handle for the menu is under the "uicontextmenu" rather than "children.
You can hack around this for now, or dive into the source if it is really bothersome.
First, delete() is the correct thing to do to remove a handle so
will rid Octave of the uicontextmenu. The 'uicontextmenu' property expects a graphics_handle to be entered. This is a double value, where NaN has the special meaning of invalid. At some point, libinterp/corefcn/graphics.in.h should be modified to accept an empty matrix input ([]). For now, this will safely rid the figure of the now deleted uicontextmenu.
As a workaround that combines these two things you could write a special deletefcn to be called whenever a uicontextmenu is deleted.
Now you can write
Since uicontextmenu is an m-file in scripts/gui this could be made a default part of Octave. Or the C++ deletion code in graphics.in.h could be improved. The destructor ~uicontextmenu is currently empty. Maybe that is the place to check the parent and see if there is a 'uicontextmenu' property and delete it, rather than in an m-file callback.
|
Fri 10 Apr 2015 02:51:08 PM UTC, original submission:
There doesn't seem to be a clean way to remove a context menu from a figure:
|