| msg26765 - (view) |
Author: Sverker Nilsson (svenil) |
Date: 2005-10-30 21:49 |
| Tkinter.Menu.delete does not delete the commands defined for the entries it deletes. Those objects will be retained until the menu itself is deleted. For example, after code like this: button = Menubutton(root, text='Window') menu = Menu(button) button['menu'] = menu def command(): print 'command button pressed' menu.add_command(command=command) menu.delete(END) del command the command function will still be referenced and kept in memory - until the menu object itself is destroyed. This may not always be a serious problem, but in my case the menu was a 'Window' menu and the command was a method on a window top level widget, so retaining a pointer to it after deleting the menu entry kept a reference to that entire window, with any associated data. I have figured out a possible fix that is in the attached file test_menuleak.py that contains some test functions. I also changed the comment - for as far as I can see, the second optional index is actually INCLUDED in the range of entries deleted. Version info Python 2.3.3 (#2, Mar 11 2004, 19:45:43) [GCC 2.95.2 20000220 (Debian GNU/Linux)] on linux2 I think it applies to all versions: I tested with the latest 2.4.2 as well. Sverker Nilsson |
|
|
| msg70550 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-01 12:19 |
| The problem does still exist (Python 2.6b2). I attached a patch for Tkinter.py which addresses this problem. It is the same as in the first proposed fix, but adds an additional check whether the menu item has a 'command' property. I also removed the "INDEX2 (is included)" comment, as it is not the desired behavior (see http://www.tcl.tk/man/tcl8.5/TkCmd/text.htm#M98). I cannot confirm the behavior, but it should be a separate issue nevertheless. |
|
|
| msg70831 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2008-08-07 14:36 |
| The patch is fine, please apply (also to the 2.5 and 3.0 branches). Don't forget a Misc/NEWS entry. |
|
|
| msg70971 - (view) |
Author: Robert Schuppenies (schuppenies) * |
Date: 2008-08-10 11:32 |
| Fixed in r65622. Backported to the release25-maint and merged into the py3k branch. |
|
|
| msg71243 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2008-08-16 22:04 |
| Uhm, this patch can cause trouble if not adapted a bit. The index method may return None for either index1 or index2, which will cause a TypeError in that for loop. If code is needed to confirm this, try the following: menu = Tkinter.Menu(tearoff=False) menu.delete(0) |
|
|
| msg71348 - (view) |
Author: Robert Schuppenies (schuppenies) * |
Date: 2008-08-18 16:29 |
| You are right. How about the attached patch, do you see any problems here? Tkinter seems to ignore any delete calls when either of the indices is None, so the deletion of commands may be ignored as well. But I couldn't find a description making this API behavior official. And does anybody know about a test suite for the Tkinter library where issues like these are tested? |
|
|
| msg71371 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2008-08-18 20:08 |
| You could return if in that new if statement. As you noted, the None argument is ignored there, that is because _tkinter checks for a None parameter, and if it happens to be a None, it then stops processing new arguments, so this is not directly related to the delete command of the Menu widget. Regarding a test suite.. it would be very nice to get one for Tkinter. Some weeks ago I did some googling but found no attempts for my surprise. |
|
|
| msg71372 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2008-08-18 20:09 |
| change this: "You could return if in that new if statement." to: "You could return in that new if statement.", please. |
|
|
| msg71446 - (view) |
Author: Robert Schuppenies (schuppenies) * |
Date: 2008-08-19 16:58 |
| I was thinking about returning in that new if statement, too, but decided not too. The reason is that I didn't want to anticipate _tkinter implementations, which may change (although not likely, still possible). Also, with the third beta tomorrow, I am not sure if somebody will find the time to approve the patch in time. |
|
|
| msg71447 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2008-08-19 17:05 |
| If this needs approval of someone else, and such thing doesn't happen in time then the earlier patch should be reverted. |
|
|
| msg71604 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2008-08-21 03:33 |
| Well, beta3 was released and the problem remained there. Robert, I believe MvL assigned this to you for a reason.. I'm a bit stressed but what I'm trying to say is that you could have decided about this issue yourself and you decided to keep the broken patch there. |
|
|
| msg71610 - (view) |
Author: Robert Schuppenies (schuppenies) * |
Date: 2008-08-21 07:29 |
| I am sry that you see it that way, I do not. I was given commit access solely for gsoc purposes and committing changes before a late release without review from a committer IMHO violates strict policies. I tried to get somebody to review the patch twice on the #python-dev channel, but was ignored. Maybe I should have made more fuss about it. Also, since it is still a beta, it is not the end of the world. I don't like it either but take the blame now. |
|
|
| msg71674 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-08-21 20:05 |
| I think the new patch looks fine and should be applied. |
|
|
| msg71729 - (view) |
Author: Robert Schuppenies (schuppenies) * |
Date: 2008-08-22 08:29 |
| Fixed in r65971. Backported to the release25-maint and merged into the py3k branch. |
|
|