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: Quentin Mathé
Subject: Re: [PATCH] Minor fix in NSWindowController
Date: Thu, 20 Apr 2006 00:48:30 +0200

Le 19 avr. 06 à 02:36, Fred Kiefer a écrit :

Quentin Mathé wrote:

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?

I have done new tests with other applications today like Notebook, Ink and ToolbarExample (which is document based). With the two last ones, the result is a segmentation fault on a document window close :-/. It's retain/release issue. Now I think that's the extra 'retain' I removed in NSWindowController is just covering another bug (may be more). I would say it's probably related to - isReleasedWhenClosed value not properly ignored everywhere when NSWindowController is used in NSDocument architecture context. What's your take on this ?

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

Yes, I just forgot to look for 'ASSIGN' in the code.

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

You are right, both problems are quite similar, I think bug 4840 isn't reintroduced by this extra retain call because _window ivar is set to nil and the code is probably checking this ivar (or the related NSDocument instance) in order to know whether any documents are still open (before displaying the 'save' alert).

In the bug report, you made the following comment : 'This behaviour is actually correct, as long as the window hasn't set isReleasedWhenClosed. Then the document and the window controller and the window will hang around invisible. This does not make any sense to me, but is acording to the spec.' Interestingly it seems Apple finally takes in account this border case. In the latest Cocoa documentation, it is stated that - isReleasedWhenClosed value is ignored when you are using a window controller together with NSDocument and related classes.

Quentin.

--
Quentin Mathé
address@hidden





reply via email to

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