gnustep-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Minor fix in NSWindowController


From: Fred Kiefer
Subject: Re: [PATCH] Minor fix in NSWindowController
Date: Wed, 19 Apr 2006 02:36:54 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20050921

Hi Quentin,

Quentin Mathé wrote:
> Le 28 mars 06 à 00:57, Fred Kiefer a écrit :
>> 
>> 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.
> 
> 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].
> 

Oops, I must admit that I don't have any memory of this change any more.
If your new code works for document based applications as well as for
others, then it should be fine to submit it. Could you please test with
Ink as well?

> 
> 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).
> 

This is not completly true, the retaining and releasing of _window gets
done in setWindow:. This seems to have been the core of that patch, to
move all code which changes the _window ivar into one place. This change
was related to the bug report #4840, which was about a similar problem
as the one you are facing now. If I only could remember...

Fred




reply via email to

[Prev in Thread] Current Thread [Next in Thread]