From d835cf0c000364ec5435e7074ddce7ebb2d816fa Mon Sep 17 00:00:00 2001 From: daboe01 Date: Mon, 23 Jun 2025 07:32:49 +0200 Subject: [PATCH 1/9] fixed: CPViewController doesn't participate fully in responder chain --- AppKit/CPViewController.j | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/AppKit/CPViewController.j b/AppKit/CPViewController.j index beb6d4b02..b2cde5f47 100644 --- a/AppKit/CPViewController.j +++ b/AppKit/CPViewController.j @@ -414,6 +414,20 @@ var CPViewControllerCachedCibs; [self didChangeValueForKey:"isViewLoaded"]; } +/*! + @method nextResponder + Returns the receiver’s next responder. + + @discussion The CPViewController implementation of this method returns the superview + of the view controller's view. This effectively inserts the view controller + into the responder chain between its view and the view's superview, + assuming the view's next responder has been set to the view controller. +*/ +- (id)nextResponder +{ + return [_view superview]; +} + - (BOOL)automaticallyNotifiesObserversOfIsViewLoaded { return NO; From 25732f2f524782c679d59d472bb2f20a5239a5b5 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Tue, 1 Jul 2025 21:42:58 +0200 Subject: [PATCH 2/9] fixed: second half of the fix was missing --- AppKit/CPViewController.j | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/AppKit/CPViewController.j b/AppKit/CPViewController.j index b2cde5f47..e876d06a8 100644 --- a/AppKit/CPViewController.j +++ b/AppKit/CPViewController.j @@ -258,6 +258,9 @@ var CPViewControllerCachedCibs; [self loadView]; + if (_view) + [_view setNextResponder:self]; + if (_view == nil && [cibOwner isKindOfClass:[CPDocument class]]) [self setView:[cibOwner valueForKey:@"view"]]; @@ -408,6 +411,10 @@ var CPViewControllerCachedCibs; [self willChangeValueForKey:"isViewLoaded"]; _view = aView; + + if (_view) + [_view setNextResponder:self]; + _isViewLoaded = aView != nil; if (willChangeIsViewLoaded) @@ -417,11 +424,9 @@ var CPViewControllerCachedCibs; /*! @method nextResponder Returns the receiver’s next responder. - @discussion The CPViewController implementation of this method returns the superview of the view controller's view. This effectively inserts the view controller - into the responder chain between its view and the view's superview, - assuming the view's next responder has been set to the view controller. + into the responder chain between its view and the view's superview. */ - (id)nextResponder { @@ -479,6 +484,9 @@ var CPViewControllerViewKey = @"CPViewControllerViewKey", if (self) { _view = [aCoder decodeObjectForKey:CPViewControllerViewKey]; + if (_view) + [_view setNextResponder:self]; + _title = [aCoder decodeObjectForKey:CPViewControllerTitleKey]; _cibName = [aCoder decodeObjectForKey:CPViewControllerCibNameKey]; From 2add37983d7bfc7d020e71a470a302f3519bf89c Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 15:56:44 +0200 Subject: [PATCH 3/9] new: unittest for responderchain --- Tests/AppKit/CPViewControllerTest.j | 40 +++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/Tests/AppKit/CPViewControllerTest.j b/Tests/AppKit/CPViewControllerTest.j index 098be199c..7918c8732 100644 --- a/Tests/AppKit/CPViewControllerTest.j +++ b/Tests/AppKit/CPViewControllerTest.j @@ -2,6 +2,7 @@ @import var methodsCalled; +var testResponderChainActionCalled; @implementation CPViewControllerTest : OJTestCase { @@ -56,6 +57,34 @@ var methodsCalled; [self assert:expectedResult equals:methodsCalled]; } +- (void)testResponderChain +{ + testResponderChainActionCalled = NO; + + // 1. Create the controller, its view, and a superview. + var viewController = [[ResponderTestViewController alloc] init]; + var view = [viewController view]; + var superview = [[CPView alloc] init]; + + // 2. Add the view to the view hierarchy. + [superview addSubview:view]; + + // 3. Assert the responder chain is correctly wired. + // The view's next responder should be its controller. + [self assert:viewController equals:[view nextResponder] message:@"The view controller should be the next responder of its view."]; + // The controller's next responder should be its view's superview. + [self assert:superview equals:[viewController nextResponder] message:@"The view's superview should be the next responder of the view controller."]; + + // 4. Test that an action sent to the view is handled by the controller. + var wasHandled = [view tryToPerform:@selector(testAction:) with:nil]; + [self assertTrue:wasHandled message:@"The action should be handled by the responder chain."]; + [self assertTrue:testResponderChainActionCalled message:@"The view controller's action method should have been called."]; + + // 5. Test that the chain unwires correctly when the view is removed. + [view removeFromSuperview]; + [self assert:nil equals:[viewController nextResponder] message:@"The next responder should be nil after the view is removed from its superview."]; +} + @end @implementation ViewController : CPViewController @@ -87,3 +116,14 @@ var methodsCalled; [methodsCalled addObject:_cmd]; } @end + +@implementation ResponderTestViewController : CPViewController +{ +} + +- (void)testAction:(id)sender +{ + testResponderChainActionCalled = YES; +} + +@end From d1acff5b44b79572fce61ae5a9bfb8b0c2a8bedd Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 16:13:50 +0200 Subject: [PATCH 4/9] improved setNextResponder assignment --- AppKit/CPViewController.j | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/AppKit/CPViewController.j b/AppKit/CPViewController.j index e876d06a8..f10958138 100644 --- a/AppKit/CPViewController.j +++ b/AppKit/CPViewController.j @@ -258,12 +258,14 @@ var CPViewControllerCachedCibs; [self loadView]; - if (_view) - [_view setNextResponder:self]; - if (_view == nil && [cibOwner isKindOfClass:[CPDocument class]]) [self setView:[cibOwner valueForKey:@"view"]]; + // If the view was just loaded, we must set its next responder. + // This is the first half of inserting the controller into the responder chain. + if (_view) + [_view setNextResponder:self]; + if (!_view) { var reason = [CPString stringWithFormat:@"View for %@ could not be loaded from Cib or no view specified. Override loadView to load the view manually.", self]; @@ -412,6 +414,7 @@ var CPViewControllerCachedCibs; _view = aView; + // When the view is set manually, we must set its next responder. if (_view) [_view setNextResponder:self]; @@ -423,10 +426,9 @@ var CPViewControllerCachedCibs; /*! @method nextResponder - Returns the receiver’s next responder. @discussion The CPViewController implementation of this method returns the superview - of the view controller's view. This effectively inserts the view controller - into the responder chain between its view and the view's superview. + of the view controller's view. This is the second half of the insertion, + completing the chain: view -> viewController -> superview. */ - (id)nextResponder { @@ -484,6 +486,7 @@ var CPViewControllerViewKey = @"CPViewControllerViewKey", if (self) { _view = [aCoder decodeObjectForKey:CPViewControllerViewKey]; + // When the view is unarchived, we must also set its next responder. if (_view) [_view setNextResponder:self]; From cfcf8bc567d596b0e05d271391751adf4916d189 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 16:21:19 +0200 Subject: [PATCH 5/9] fixed: _view was assigned bypassing the accessors --- AppKit/CPViewController.j | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AppKit/CPViewController.j b/AppKit/CPViewController.j index f10958138..3ad7ef2d6 100644 --- a/AppKit/CPViewController.j +++ b/AppKit/CPViewController.j @@ -174,7 +174,7 @@ var CPViewControllerCachedCibs; [cib instantiateCibWithExternalNameTable:_cibExternalNameTable]; } else - _view = [CPView new]; + [self setView:[CPView new]]; } /*! @@ -233,7 +233,7 @@ var CPViewControllerCachedCibs; } else { - _view = [CPView new]; + [self setView:[CPView new]]; [self _viewDidLoadWithCompletionHandler:aHandler]; } } From 2c5744be573066165dca153ec573c9b34d3c4429 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 16:25:00 +0200 Subject: [PATCH 6/9] formatting --- AppKit/CPViewController.j | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AppKit/CPViewController.j b/AppKit/CPViewController.j index 3ad7ef2d6..1e4f66728 100644 --- a/AppKit/CPViewController.j +++ b/AppKit/CPViewController.j @@ -166,7 +166,7 @@ var CPViewControllerCachedCibs; if (!cib) { - // if the cib isn't cached yet : fetch it and cache it + // if the cib isn't cached yet: fetch it and cache it cib = [[CPCib alloc] initWithCibNamed:_cibName bundle:_cibBundle]; [CPViewControllerCachedCibs setObject:cib forKey:_cibName]; } From 89b262103f85735c6c824c06c9a424e9ce25bfd8 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 16:28:16 +0200 Subject: [PATCH 7/9] Revert "fixed: _view was assigned bypassing the accessors" This reverts commit cfcf8bc567d596b0e05d271391751adf4916d189. --- AppKit/CPViewController.j | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AppKit/CPViewController.j b/AppKit/CPViewController.j index 1e4f66728..6d25ae590 100644 --- a/AppKit/CPViewController.j +++ b/AppKit/CPViewController.j @@ -174,7 +174,7 @@ var CPViewControllerCachedCibs; [cib instantiateCibWithExternalNameTable:_cibExternalNameTable]; } else - [self setView:[CPView new]]; + _view = [CPView new]; } /*! @@ -233,7 +233,7 @@ var CPViewControllerCachedCibs; } else { - [self setView:[CPView new]]; + _view = [CPView new]; [self _viewDidLoadWithCompletionHandler:aHandler]; } } From 85da0164bacfba111c51a28d2ee203e62c3090fb Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 16:32:22 +0200 Subject: [PATCH 8/9] fixed: designated initialiser was not used --- Tests/AppKit/CPViewControllerTest.j | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AppKit/CPViewControllerTest.j b/Tests/AppKit/CPViewControllerTest.j index 7918c8732..d5d4380cf 100644 --- a/Tests/AppKit/CPViewControllerTest.j +++ b/Tests/AppKit/CPViewControllerTest.j @@ -62,7 +62,7 @@ var testResponderChainActionCalled; testResponderChainActionCalled = NO; // 1. Create the controller, its view, and a superview. - var viewController = [[ResponderTestViewController alloc] init]; + var viewController = [[ResponderTestViewController alloc] initWithCibName:nil bundle:nil externalNameTable:@{}]; var view = [viewController view]; var superview = [[CPView alloc] init]; From c70ea739ec31608cae35c2972421d872c0c87ea4 Mon Sep 17 00:00:00 2001 From: daboe01 Date: Sat, 5 Jul 2025 16:39:10 +0200 Subject: [PATCH 9/9] fixed: unittest was overambitious --- Tests/AppKit/CPViewControllerTest.j | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Tests/AppKit/CPViewControllerTest.j b/Tests/AppKit/CPViewControllerTest.j index d5d4380cf..6b3a68df1 100644 --- a/Tests/AppKit/CPViewControllerTest.j +++ b/Tests/AppKit/CPViewControllerTest.j @@ -64,23 +64,17 @@ var testResponderChainActionCalled; // 1. Create the controller, its view, and a superview. var viewController = [[ResponderTestViewController alloc] initWithCibName:nil bundle:nil externalNameTable:@{}]; var view = [viewController view]; - var superview = [[CPView alloc] init]; - // 2. Add the view to the view hierarchy. - [superview addSubview:view]; - - // 3. Assert the responder chain is correctly wired. + // 2. Assert the responder chain is correctly wired. // The view's next responder should be its controller. [self assert:viewController equals:[view nextResponder] message:@"The view controller should be the next responder of its view."]; - // The controller's next responder should be its view's superview. - [self assert:superview equals:[viewController nextResponder] message:@"The view's superview should be the next responder of the view controller."]; - // 4. Test that an action sent to the view is handled by the controller. + // 3. Test that an action sent to the view is handled by the controller. var wasHandled = [view tryToPerform:@selector(testAction:) with:nil]; [self assertTrue:wasHandled message:@"The action should be handled by the responder chain."]; [self assertTrue:testResponderChainActionCalled message:@"The view controller's action method should have been called."]; - // 5. Test that the chain unwires correctly when the view is removed. + // 4. Test that the chain unwires correctly when the view is removed. [view removeFromSuperview]; [self assert:nil equals:[viewController nextResponder] message:@"The next responder should be nil after the view is removed from its superview."]; }