Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when presenting an UIActivityViewController in popover on iPad #22

Open
Bluezen opened this issue Nov 1, 2014 · 11 comments
Open
Labels
Milestone

Comments

@Bluezen
Copy link

Bluezen commented Nov 1, 2014

Hello,
First of all, thanks for maintaining this project!

I encounter a crash when touching the "More" button of an UIActivityViewController presented in a system popover on iPad with iOS8.

ios simulator screen shot 1 nov 2014 15 13 33

Crash happens in WYPopoverController.m file:
crash

You can reproduce the crash by using my modified version of the demo project (I simply added a toolbar with a button to present a UIActivityViewController 4f2e755) on my fork:
https://github.com/Bluezen/WYPopoverController/tree/demoCrash
You have to run the demo project on an iPad (real device or simulator) with iOS8, touch the button on the toolbar then touch the "More" button.

I still can't figure out why there is a EXC_BAD_ACCESS here so I'm using a "ghetto fix" for the moment:

if ([self isKindOfClass:[UINavigationController class]] == NO && self.navigationController != nil && [self respondsToSelector:@selector(popoverPresentationController)] == NO)
    {

but this doesn't solve the underlying problem and is just one ugly way to work around it.

I hope someone investigation will prove better than mine.

@keremerkan
Copy link

I have encountered the exact same problem and I am not even using WYPopoverController to present the ActivityViewController. I am using the system provided UIPopoverController. This is strange.

@keremerkan
Copy link

Ah, I did not see that that method was a UIViewController category method. That's why it is called even though I am not using WYPopoverController. Your temporary fix seems to be working on iOS 8.1 @Bluezen

@Bluezen
Copy link
Author

Bluezen commented Nov 6, 2014

Yes, you are describing the exact same problem.
It is indeed because WYPopoverController swizzle the setPreferredContentSize:method of UIViewController that the crash occurs even though we are not explicitly using WYPopoverController.

My temporary solution is simply a safeguard, it prevents the faulty line to be called on iOS8 and could yield bad side effects. I chose to ignore it because I never use a UINavigationController in my WYPopovers... so be careful, this is not a fix!

Ok, after spending too much time again on it I might have a better idea of what the real problem is and much more importantly, I have a better understanding of what sizzled_setPreferredContentSize does.

I wrongly assumed that there was an error in sizzled_setPreferredContentSize but we are instead facing an unexpected side effect of messing with a widely used system method.

The recursive call that

[self.navigationController setPreferredContentSize:aSize];

creates is intended and

if ([self isKindOfClass:[UINavigationController class]] == NO && self.navigationController != nil)

is the safeguard that should stop it to loop indefinitely.

It seems that

[self.navigationController setPreferredContentSize:aSize];

is a necessary call to be able to resize a WYPopoverController dynamically but unfortunately the safeguard doesn't play its role well when a system UIPopover with a UINavigationController is the one to yield the call.

We have to understand why and how to stop the infinite loop in this case.

@Bluezen
Copy link
Author

Bluezen commented Nov 6, 2014

Ok, what is happening is very strange. The issue has something to do with the fact that we are dealing with private classes. In the internal hierarchy of a presented UIActivityViewController there is what seems to be a subclass of UINavigationController called _UIUserDefaultsActivityNavigationController (visible after pushing the "more" button).

In our situation the erroneous call starting the infinite loop happens in sizzled_setPreferredContentSize when self is of kind _UIActivityUserDefaultsViewController and self.navigationController an instance of kind _UIUserDefaultsActivityNavigationController.

The thing I cannot explain is what happen in this precise situation when the following message is sent:

[self.navigationController setPreferredContentSize:aSize];

The swizzled function sizzled_setPreferredContentSize is called, which seems normal, but we are in the exact same configuration than before; self is the same instance of _UIActivityUserDefaultsViewController than before instead of being the instance of navigationController _UIUserDefaultsActivityNavigationController. This causes the infinite loop.

I can only guess that the bizarre behavior is because the _UIUserDefaultsActivityNavigationController is internally forwarding the setPreferredContentSize to our instance of _UIActivityUserDefaultsViewController, hence the exact same configuration and the loop... or something like that.

Anyway, the best solution I can come up with for the moment is to prevent the call to be made only in this precise situation:

- (void)sizzled_setPreferredContentSize:(CGSize)aSize
{
    [self sizzled_setPreferredContentSize:aSize];

    BOOL isPrivateUIActivityNavigationController = [NSStringFromClass(self.navigationController.class) isEqualToString:@"_UIUserDefaultsActivityNavigationController"];

    if ([self isKindOfClass:[UINavigationController class]] == NO
        && self.navigationController != nil
        && isPrivateUIActivityNavigationController == NO)
    {
#ifdef WY_BASE_SDK_7_ENABLED
        if ([self respondsToSelector:@selector(setPreferredContentSize:)]) {
            [self.navigationController setPreferredContentSize:aSize];
        }
#endif
    }
}

That prevents the crash to happen but doesn't break dynamic resize of WYPopover presenting a UINavigationController.

@keremerkan
Copy link

Well, this still is not a perfect solution as you said, though thank you very much for trying to get to the bottom of the problem. This can happen on other private classes, so we need to come up with a better system.

@Bluezen
Copy link
Author

Bluezen commented Nov 7, 2014

Yeah, you are right, this could happen with other private classes.
Maybe we should take the problem backwards, instead of preventing private classes to make a mess we should specify to do nothing if we are not dealing with a WYPopover.

@keremerkan
Copy link

Yes, that's exactly what I was trying to say. Adding categories on UIViewController is a little bit dangerous IMHO.

@Bluezen
Copy link
Author

Bluezen commented Nov 8, 2014

I agree with you, if possible we should avoid swizzle such methods of UIViewController. Maybe this should be refactored to be handled differently.
In the meantime I am suggesting this:

- (void)sizzled_setPreferredContentSize:(CGSize)aSize
{
    [self sizzled_setPreferredContentSize:aSize];

    if ([self isKindOfClass:[UINavigationController class]] == NO && self.navigationController != nil)
    {
#ifdef WY_BASE_SDK_7_ENABLED
        if ([self.navigationController isEmbedInPopover] == NO) {
            return;
        } else if ([self respondsToSelector:@selector(setPreferredContentSize:)]) {
            [self.navigationController setPreferredContentSize:aSize];
        }
#endif
    }
}

hsoi added a commit to barz/WYPopoverController that referenced this issue Nov 22, 2014
@YoniTsafir
Copy link

👍 This happens to me too, when will a fix be released?

@vitalys vitalys added the bug label Feb 3, 2016
@mingshing
Copy link

The pull request in the original repo will resolve this issue.
nicolaschengdev#182

@vitalys
Copy link
Collaborator

vitalys commented Aug 29, 2016

I'll check this issue tomorrow

@vitalys vitalys added this to the 0.4.0 milestone Aug 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants