Also, it's worth noting this problem only shows up in in-window menu mode (with or without the GNOME theme)
GC On Sat, Jun 4, 2011 at 11:55 AM, Gregory Casamento <[email protected]> wrote: > The application showing the issue currently is Gorm. The only other > application I've tried is Ink and the problem is not showing there. > > Here's the backtrace: > > #0 0xb761471c in objc_msg_lookup () from /usr/lib/libobjc.so.2 > #1 0xb7b3492d in -[NSApplication targetForAction:to:from:] > (self=0x81614f0, _cmd=0xb7e1fbe0, theAction=0xb7dd1938, > theTarget=0x84f37a0, sender=0x886bdb8) at NSApplication.m:2248 > #2 0xb7bfc2be in -[NSMenu update] (self=0x8925d78, _cmd=0xb7e1fb18) > at NSMenu.m:1157 > #3 0xb7bfc50c in -[NSMenu update] (self=0x8999e18, _cmd=0xb7ea19d0) > at NSMenu.m:1152 > #4 0xb7d36e19 in -[GSTheme(Menu) setMenu:forWindow:] (self=0x8299d58, > _cmd=0xb7ea1a28, menu=0x82866b0, window=0x8954ab8) at GSThemeMenu.m:91 > #5 0xb7d371a6 in -[GSTheme(Menu) updateMenu:forWindow:] > (self=0x8299d58, _cmd=0xb7ea1a50, menu=0x82866b0, window=0x8954ab8) at > GSThemeMenu.m:149 > #6 0xb7d3729c in -[GSTheme(Menu) updateAllWindowsWithMenu:] > (self=0x8299d58, _cmd=0xb7e1fc28, menu=0x82866b0) at GSThemeMenu.m:162 > #7 0xb7bfc5bf in -[NSMenu update] (self=0x82866b0, _cmd=0xb7dd16f0) > at NSMenu.m:1216 > #8 0xb7b34134 in -[NSApplication run] (self=0x81614f0, > _cmd=0xb7dc7268) at NSApplication.m:1567 > #9 0xb7b1b9ca in NSApplicationMain (argc=1, argv=0xbfffefc4) at > Functions.m:89 > #10 0x0804f8cb in main (argc=1, argv=0xbfffefc4) at main.m:30 > > On Sat, Jun 4, 2011 at 6:10 AM, Fred Kiefer <[email protected]> wrote: >> I see now that your change is again just a workaround. Would you mind to add >> a comment, next time you add code that is most likely wrong? >> >> It surely would help to track down the real problem if you could share and >> application showing the issue or at least a back trace of the problem. >> >> I had a short look at NSMenuItem and we seem to use the ivar _target for two >> separate purposes. For the normal target of the menu item and as well for >> the super menu, when the item holds a submenu. Could you please try to find >> out, which of these two cases is the problematic one? You could do the >> retain just in one of the cases (target being an NSMenu or not) and see >> whether you get the problem. If it isn't the NSMenu target, then I would >> expect that problem to be in a completely different place in your >> application. Here a static code analysis or a memory debugger could help. >> >> I really would like to see this problem resolved this time. The change you >> made looks wrong and somebody may remove it again at some point, thereby >> breaking your application. >> >> Fred >> >> On 03.06.2011 10:19, Gregory Casamento wrote: >>> >>> 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? >> >> > > > > -- > Gregory Casamento - GNUstep Lead/Principal Consultant, OLC, Inc. > yahoo/skype: greg_casamento, aol: gjcasa > (240)274-9630 (Cell) > -- 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
