Re: [PATCH] Minor fix in NSWindowController

[Quentin Mathé]

Le 28 mars 06 à 00:57, Fred Kiefer a écrit :

> Hi Quentin,
>
> I am rather unsure about this patch.I don't quite remember, who did  
> put
> in these commetn lines on retaining the window, but I would expect  
> that
> there were reasons for it at that time. Still they may have been  
> the now
> redundant solution of an already resolved problem.

Hi Fred,

I think you wrote the comments, here is the ChangeLog entry which  
introduced this 'retain' line :
2003-08-30  Fred Kiefer <address@hidden>

        * Source/NSWindowController.m
        Changed [setWindow:] to manage the notification connection to the
        window.
        [initWithWindow:], [dealloc] and [_windowWillClose:] now use
        [setWindow:].
        [setDocument:] now no longer retains the document, as this is
        already retaining the window controller. Simplified [loadWindow]
        by using the method [windowNibPath].

You can take a look at the related diff with this link : <http:// 
svn.gna.org/viewcvs/gnustep/libs/gui/trunk/Source/ 
NSWindowController.m?r2=17574&rev=17574&r1=16917&dir_pagestart=150>

The previous comment makes a bit more sense (unlike the most recent  
one) :

           /*
            * If the window is set to isReleasedWhenClosed, it will  
release
            * itself, so nil out our reference so we don't release  
it again
            *
            * Apple's implementation doesn't seem to deal with this  
case, and
            * crashes if isReleaseWhenClosed is set.
            */
            */
           _window = nil;

However I think it's not needed because there is no 'retain' or  
'release' calls on _window in NSWindowController.m (not even in - 
dealloc method).

> Did you do sufficent tests with your patch? As there aren't that many
> document based applications for GNUstep, we really shoud try all of
> them, before releasing the patch.

I have done far more extensive tests right now and encountered no  
issues at all with applications like ProjectCenter, Project Manager,  
Gorm (going to try a few more).

Thanks,
Quentin.

--
Quentin Mathé
address@hidden