jessesquires / JSQMessagesViewController

An elegant messages UI library for iOS
https://www.jessesquires.com/blog/officially-deprecating-jsqmessagesviewcontroller/
Other
11.14k stars 2.82k forks source link

iOS 9 crash when long pressing on links and view pushed modally #1247

Closed chrisellsworth closed 8 years ago

chrisellsworth commented 9 years ago

Try the demo code, revision 49135c660a851d395ce851483b4b2ed2833af5e0 on iOS 9 to reproduce this.

2015-10-07 17:10:56.189 JSQMessages[80242:1113252] Warning: Attempt to present <_UIRotatingAlertController: 0x7ffd81037e00> on <UINavigationController: 0x7ffd81035200> whose view is not in the window hierarchy!

2015-10-07 17:11:22.515 JSQMessages[80242:1113252] *** Assertion failure in -[JSQMessagesCellTextView startInteractionWithLinkAtPoint:], /BuildRoot/Library/Caches/com.apple.xbs/Sources/UIKit_Sim/UIKit-3505.16/UITextView_LinkInteraction.m:377
2015-10-07 17:11:22.518 JSQMessages[80242:1113252] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: ''
*** First throw call stack:
(
    0   CoreFoundation                      0x0000000109d9df65 __exceptionPreprocess + 165
    1   libobjc.A.dylib                     0x0000000109817deb objc_exception_throw + 48
    2   CoreFoundation                      0x0000000109d9ddca +[NSException raise:format:arguments:] + 106
    3   Foundation                          0x0000000107839ae2 -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 198
    4   UIKit                               0x00000001085eeaf0 -[UITextView(LinkInteraction) startInteractionWithLinkAtPoint:] + 204
    5   UIKit                               0x00000001080da2dd -[UITextInteractionAssistant(UITextInteractionAssistant_Internal) smallDelayRecognizer:] + 580
    6   UIKit                               0x00000001080c8b40 _UIGestureRecognizerSendTargetActions + 153
    7   UIKit                               0x00000001080c56af _UIGestureRecognizerSendActions + 162
    8   UIKit                               0x00000001080c3f01 -[UIGestureRecognizer _updateGestureWithEvent:buttonEvent:] + 822
    9   UIKit                               0x00000001080cb3f3 ___UIGestureRecognizerUpdate_block_invoke809 + 79
    10  UIKit                               0x00000001080cb291 _UIGestureRecognizerRemoveObjectsFromArrayAndApplyBlocks + 342
    11  UIKit                               0x00000001080bceb4 _UIGestureRecognizerUpdate + 2624
    12  CoreFoundation                      0x0000000109cc99d7 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    13  CoreFoundation                      0x0000000109cc9947 __CFRunLoopDoObservers + 391
    14  CoreFoundation                      0x0000000109cbf59b __CFRunLoopRun + 1147
    15  CoreFoundation                      0x0000000109cbee98 CFRunLoopRunSpecific + 488
    16  GraphicsServices                    0x000000010aac3ad2 GSEventRunModal + 161
    17  UIKit                               0x0000000107c0c676 UIApplicationMain + 171
    18  JSQMessages                         0x000000010725d72f main + 111
    19  libdyld.dylib                       0x000000010ac0792d start + 1
)
libc++abi.dylib: terminating with uncaught exception of type NSException
jessesquires commented 9 years ago

Looks like an iOS 9 UIKit bug. Will investigate soon.

sukhrobkhakimov commented 9 years ago

It is a iOS 9 bug. This is a workaround:

- (BOOL)textView:(UITextView *)textView shouldInteractWithURL:(NSURL *)URL inRange:(NSRange)characterRange {
    if (IOS_9) {
        [[UIApplication sharedApplication] openURL:URL]; // Fix for iOS 9 bug

        return NO;
    }

    return YES;
}
jessesquires commented 9 years ago

Thanks @Sukhrob !

jessesquires commented 8 years ago

Hm. This doesn't seem to fix it.

BillCarsonFr commented 8 years ago

It's very strange, the crash occurs the second time. First time you have this error:

Warning: Attempt to present <_UIRotatingAlertController: 0x1588ae200> on <UINavigationController: 0x158069c00> whose view is not in the window hierarchy!

Second time

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: ''

Here is the workaround. In your MessagesViewController, override cellForItemAtIndexPath

 override public func collectionView(collectionView: UICollectionView, cellForItemAtIndexPath indexPath: NSIndexPath) -> UICollectionViewCell {
 ...
 cell.textView?.delegate = self

Then:

    public override func textView(textView: UITextView, shouldInteractWithURL URL: NSURL, inRange characterRange: NSRange) -> Bool {
    if floor(NSFoundationVersionNumber) > NSFoundationVersionNumber_iOS_8_3 && textView is JSQMessagesCellTextView {
        UIApplication.sharedApplication().openURL(URL)
        return false
    }
    return super.textView(textView, shouldInteractWithURL: URL, inRange: characterRange)
}
anfriis commented 8 years ago

Thanks @BillCarsonFr, now it doesn't crash in my app running on iOS 9 and it's possible to open URL links. But I also have addresses in my chat, which are being handled as URL's also and cannot be opened. The shouldInteractWithURL method's URL parameter is set to x-apple-data-detectors://embedded-result/53 when clicking on an address in the chat. Does anyone know a fix for that so it is possible to handle addresses also?

ryanphillipthomas commented 8 years ago

@Anders123fr Another solution is to detect a long press, and just return NO in shouldInteractWithURL for that. Whenever it's not a long press, return YES and let it use the default behavior.

So something like:

- (BOOL)textView:(UITextView *)textView shouldInteractWithURL:(NSURL *)URL inRange:(NSRange)characterRange {
    BOOL isLongPress = NO;
    for (UIGestureRecognizer *recognizer in textView.gestureRecognizers) {
        if ([recognizer isKindOfClass:[UILongPressGestureRecognizer class]]) {
            if (recognizer.state == UIGestureRecognizerStateBegan) {
                isLongPress = YES;
            }
        }
    }
    if (isLongPress) {
        return NO;
    }
    return YES;
}
Mackarous commented 8 years ago

Ok, so I looked into this one. It is a known issue and a radar has been filed. Unfortunately it is not on openradar, so we'll just have to wait and see if Apple will fix this one.

The gist (no pun intended) of the problem is when the JSQMessagesViewController's UITextView link is long pressed, it tries to present a UIAlertController action sheet. The issue here is that the action sheet is presented from the root view controller of the app. There is a limitation on iOS in that a view controller cannot be presented in the context of a UINavigationController when another UINavigationController is presented. Only one at a time please!

So, I made a workaround, but it sure is ugly. Basically I am recreating the action sheet and presenting it myself, while looping through the presented view controllers:

UIViewController *presentingViewController = [UIApplication sharedApplication].keyWindow.rootViewController;
while (presentingViewController.presentedViewController) {
    presentingViewController = presentingViewController.presentedViewController;
}
[presentingViewController presentViewController:alertController animated:true completion:nil];

I can put through a pull request shortly until Apple fixes the issue.

The app will need new localizations for the following:

Edit: Oops! Just realized I was only testing with URLs. This might be messier than I thought if I have to add a different action sheet for each link type. I am investigating another workaround

Edit 2: Found a much better alternative. Made a PR

steipete commented 8 years ago

We've recently stumbled on this issue as well in PSPDFKit and found a workaround that doesn't require recreating the action sheet. It's still somewhat horrible - but at least it's self-contained and the things it changes are local to this specific bug. The main strategy is to be selective and apply a workaround (selecting the correct view controller to present on) just in time - then clean up again. Here's the gist:

https://gist.github.com/steipete/b00fc02aa9f1c66c11d0f996b1ba1265

And please dupe rdar://26295020 so this will get hopefully fixed in time for iOS 10. (The bug exists since iOS 8 and was first reported on iOS 8b5.)

This workaround does use private symbols, so it's not great either. When we select a fix we think about the total cost and risk; recreating the menu seems like a lot of code, buggy, and will likely look/feel different. You can mask the fix and make sure it does not get executed for iOS 10+, to further reduce the risk, but really, duping the radar and hope Apple will fix up this in both places is our best option.

jessesquires commented 8 years ago

Thanks @steipete ! 😊

steipete commented 8 years ago

Hope you didn't miss my comment on https://github.com/jessesquires/JSQMessagesViewController/commit/c4143c59a541d7c6b487396fa3135b88dba34358#commitcomment-17575574 - else you'll risk people's app being rejected.

jessesquires commented 8 years ago

Derp! It was late last night 😊 Thanks so much @steipete ! 🙌 🙇

ndonald2 commented 7 years ago

The swizzled "fix" here breaks a few the long press menu action sheet options for detected data links on non force-touch devices running iOS 10 (though oddly not in the simulator). The system-presented action sheet does not properly dismiss when choosing "Send Message" or "Add to Contacts" after long pressing a phone number or email, and the nav bar in the presented message/contacts controller is messed up - seems to be transparent. Here's a screenshot of what happens to the action sheet after dismissing the "Add to Contacts" controller.

img_1727

I'm not familiar with the original iOS 9 bug so I can't say whether it's been fixed on iOS 10, but removing the swizzling fixes the presentation issue I've described here. I'll probably use an internal fork with the swizzling removed, but I just thought others should know.

Also, I understand the tradeoffs, but it seems somewhat unforthcoming to use private APIs via obfuscated dynamic instantiation without surfacing that to users. It might get past Apple's automatic private API detection but these types of things are prone to breaking in the long run as UIKit changes.

jessesquires commented 7 years ago

thanks @ndonald2 ! 👍

If you could open a new issue with your comments here, that would be 💯