Skip to content
This repository has been archived by the owner on Jun 8, 2023. It is now read-only.

RBLViewControllerAddition replaces wrong method. #87

Merged
merged 1 commit into from
May 24, 2013
Merged

RBLViewControllerAddition replaces wrong method. #87

merged 1 commit into from
May 24, 2013

Conversation

anozaki
Copy link
Contributor

@anozaki anozaki commented May 24, 2013

If the first view you use happens to be overloading any of the method, for example viewWillMoveToWindow, the swap function will swap out the overloaded class with the custom_ version.

The swap should replace the base implementation that is provided by the NSView. This patch will force the NSView's method to be replaced instead of the overloaded method.

@jspahrsummers
Copy link
Contributor

the swap function will swap out the overloaded class with the custom_ version.

What problems does this cause in practice?

This patch will force the NSView's method to be replaced instead of the overloaded method.

Semantically, this makes sense, but subclass overrides are not required to invoke NSView's implementation (per the documentation). This might cause the RBLViewController implementations to be skipped unless we run our code first.

@anozaki
Copy link
Contributor Author

anozaki commented May 24, 2013

For example, viewWillMoveToWindow will only get swapped once. So depending on how you're code is loaded it replaces the wrong method.

// for example if I have the following interface and implementation to be used with RBLViewController.
@interface MyInitialView : NSView
@end

@implementation MyInitialView
- (void) viewWillMoveToWindow { // <----- This line causes problem because swap would find this function.
    // ... something special we need for view or using a lib that does this
}
@end

When you use this view with RBLViewController viewWillMoveToWindow will get replaced as expected. However on you're second view you create...

@interface MySecondView : NSView
// viewWillMoveToWindow is not replaced correctly for this view.
@end

When you use this view with RBLViewController, not all the ViewController calls will be called. This is because the swap method only runs once and it ran and replaced MyInitialView's viewWillMoveToWindow. If you subclass MyInitialView, it will work as expected.

I'm not quite what you mean by RBLViewController implementation would get skipped. From what I understand RBLViewController kick starts everything.

@jspahrsummers
Copy link
Contributor

For example, viewWillMoveToWindow will only get swapped once. So depending on how you're code is loaded it replaces the wrong method.

Ah, indeed. Sorry, I misunderstood your original problem description.

In that case, the proper fix would be making sure that any used view classes receive a custom implementation of -viewWillMove… and -viewDidMove…, but that's tricky to do correctly.

I'm not quite what you mean by RBLViewController implementation would get skipped.

The documentation for -viewDidMoveToWindow says "The default implementation does nothing; subclasses can override this method to perform whatever actions are necessary."

Developers often avoid calling super for methods like that, because they expect it to do nothing, but it introduces a problem for us. Consider the following custom class:

@implementation CustomView : NSView

- (void)viewDidMoveToWindow {
    NSLog(@"%s", __func__);
}

@end

If we set up a couple view controllers:

NSView *standardView = [[NSView alloc] initWithFrame:NSZeroRect];
RBLViewController *standardVC = [RBLViewController viewController];
standardView.rbl_viewController = standardVC;

CustomView *customView = [[CustomView alloc] initWithFrame:NSZeroRect];
RBLViewController *customVC = [RBLViewController viewController];
customView.rbl_viewController = customVC;

… then -viewDidMoveToWindow will never be invoked on customVC, because the implementation in CustomView – by not invoking super – prevents our swizzled code from ever running.


Anyways, although this is a real problem, I think your fix is better than what we have currently, so I'll merge it in. Thanks! ✨

jspahrsummers added a commit that referenced this pull request May 24, 2013
RBLViewControllerAddition replaces wrong method.
@jspahrsummers jspahrsummers merged commit a765296 into github:master May 24, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants