Fred, Yes, at this point I do realize it was wrong. I'm not entirely convinced that the fix I did this time around was correct since it involves retaining the _target ivar of the item. However, the fix that is currently in place seems "more correct" than using the archiver as I was doing before.
I think there is an extraneous release someplace, but, thus far, I'm unable to find it. Later, GC On Fri, Jun 3, 2011 at 3:41 AM, Fred Kiefer <[email protected]> wrote: > Thank you that you finally took the time to inspect the real issue and fix > it. In the meantime you surely noticed that the analysis you provided below > was wrong. The NSMenu did copy the NSMenuItems. The bug was in the copy > method of the item. > > It would have been a lot better if you had looked into the issue when it > first showed up, but everything should be fine now. > > Fred > > > Am 03.06.2011 um 00:24 schrieb Gregory Casamento <[email protected]>: > >> Fred, >> >> It's actually pretty simple to see what's broken in the copyWithZone >> method in NSMenu.m. That's what I was trying to explain in the >> "Revert". I don't need to write a test program since it's plain to >> see on inspection that it's incorrect. >> >> The method does a recursive copy of the menu and simply inserts the >> menu items from the menu being copied into the copy. As I explained >> in my comment, this causes all of the menus to share all of their >> NSMenuItems, which is not correct. >> >> It wasn't my intention to do a "silent revert" or anything of that >> nature. I'm trying right now to fix some issues I'm seeing with the >> Gnome theme and had forgotten about the debate in March. I'll try to >> fix this problem properly sometime this evening. >> >> Thanks, GC >> >> On Thu, Jun 2, 2011 at 5:47 PM, Fred Kiefer <[email protected]> wrote: >>> On 02.06.2011 22:22, Gregory Casamento wrote: >>>> >>>> Author: gcasa >>>> Date: Thu Jun 2 22:22:39 2011 >>>> New Revision: 33234 >>>> >>>> URL: http://svn.gna.org/viewcvs/gnustep?rev=33234&view=rev >>>> Log: >>>> Use the NSArchiver to copy the menu since the copy method does not do a >>>> deep enough copy. >>>> >>>> Modified: >>>> libs/gui/trunk/ChangeLog >>>> libs/gui/trunk/Source/GSThemeMenu.m >>> >>> Haven't we been through that before? >>> >>> There was a long mail exchange in March why this code was incorrect and >>> causing trouble to German. At that point I suggested that you report back >>> with a short example program showing what is broken with the [NSMenu >>> -copyWithZone:] method. Instead you changed the code to use copy and now you >>> changed it back and added this comment: >>> >>> /* >>> * NOTE: The reason the copy or copyWithZone method is not used here is >>> because >>> * it doesn't make a deep copy of the menu. A deep copy is needed in order >>> to >>> * allow the individual menu items to be used in multiple menus at one time >>> without >>> * interfering with one another's state. >>> */ >>> >>> This doesn't explain anything, nor will it ever help to resolve the issue, >>> if there is any. It just breaks German's code again. >>> >>> Don't you think that reopening the debate with what ever new evidence you >>> came up with would be a lot more productive then doing a silent revert? >>> >>> _______________________________________________ >>> Gnustep-dev mailing list >>> [email protected] >>> https://lists.gnu.org/mailman/listinfo/gnustep-dev >>> >> >> >> >> -- >> Gregory Casamento - GNUstep Lead/Principal Consultant, OLC, Inc. >> yahoo/skype: greg_casamento, aol: gjcasa >> (240)274-9630 (Cell) >> >> _______________________________________________ >> Gnustep-dev mailing list >> [email protected] >> https://lists.gnu.org/mailman/listinfo/gnustep-dev > -- Gregory Casamento - GNUstep Lead/Principal Consultant, OLC, Inc. yahoo/skype: greg_casamento, aol: gjcasa (240)274-9630 (Cell) _______________________________________________ Gnustep-dev mailing list [email protected] https://lists.gnu.org/mailman/listinfo/gnustep-dev
