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

Typhoon causes crash while transferring autolayout #513

Closed
Brain89 opened this issue Jun 28, 2016 · 14 comments
Closed

Typhoon causes crash while transferring autolayout #513

Brain89 opened this issue Jun 28, 2016 · 14 comments
Assignees
Labels

Comments

@Brain89
Copy link

Brain89 commented Jun 28, 2016

Hello,

We have faced with an issue crashing our app

Fatal Exception: NSUnknownKeyException
[<NSLayoutConstraint 0x174283cf0> setValue:forUndefinedKey:]: this class is not key value coding-compliant for the key firstItem.

It appears only on devices running iOS 10 (yep, new SDK - new problems). I have attached crash log from Crashlytics.
As I understand iOS 10 SDK does not support setter for firstItem property of NSLayoutConstraint.

CrashLog.txt

@Przemyslaw-Wosko
Copy link
Contributor

how to reproduce it?

@Brain89
Copy link
Author

Brain89 commented Jul 6, 2016

To reproduce this bug you should add TyphoonLoadedView into your project and run it inside iOS X environment.

@vasilenkoigor
Copy link

vasilenkoigor commented Jul 11, 2016

Hey @Brain89
Looks strange, because NSLayoutConstraint doesn't changed in iOS 10 SDK, with the exception of added firstAnchor and secondAnchor
https://developer.apple.com/library/prerelease/content/releasenotes/General/iOS10APIDiffs/Objective-C/UIKit.html

However, I sure that crash happens in this place TyphoonViewHelpers

[constraint setValue:firstItem forKey:NSStringFromSelector(@selector(firstItem))];

Might be create issue on radar? I'm think this is SDK bug.

@Brain89
Copy link
Author

Brain89 commented Jul 12, 2016

I do not agree that this is a SDK bug. firstItem is a readonly property (see docs)

@property(readonly, assign) id firstItem

So the code to which you refer uses private API. Apple engineers could change it without any notice.

@vasilenkoigor
Copy link

@Brain89
hmm...but wait, this property is readonly since iOS 6...
nope? :)

@vasilenkoigor
Copy link

vasilenkoigor commented Jul 13, 2016

May be Apple engineers closed back-door that allows change readonly property with KVC😁
Ok, in few days I'll make workaround.

Thanks for reporting👌

@Brain89
Copy link
Author

Brain89 commented Jul 28, 2016

We must reopen this issue. Merged PR has broken #464
@CognitiveDisson will fix it.

@alexgarbarev
Copy link
Contributor

@Brain89 but how do you think this can be fixed without using "setValue:forKey" since it crashes on iOS10 that way?

@CognitiveDisson
Copy link
Contributor

@alexgarbarev Subclass of NSLayoutConstraint fixed this issue. If constraint is it subclass then transfer, else create new constraint. What do you think about this solution?

@alexgarbarev
Copy link
Contributor

So user have to choose subclass in IB while creating a constraint? For me it looks invasive. It looks like there is no legal way to modify existing constraint's first and last items.. Maybe it's better to not include support for outlets for these constraints in Typhoon, then it's more stable since uses legal APIs.
Anyone who wants TyphoonLoadedView with constraints and outlet always free to use own solution with subclasses, swizzling or anything else.. @vasilenkoigor what do you think about this problem?

@Brain89
Copy link
Author

Brain89 commented Jul 28, 2016

Well, without creating NSLayoutContraint subclass inside library, every Typhoon user with constraint outlets necessity (like in #464) will have to create his own Typhoon fork, where it will be possible to fix this issue (create subclass and improve ViewHelpers in a way @CognitiveDisson wants). Seems very strange too.

@CognitiveDisson
Copy link
Contributor

@alexgarbarev If user not use IB to TyphoonLoadedView, then he does not need set subclass. (And all be works like now) But if the user will need this functionality, it will only need to specify the subclass.
Another solution is swizzling. (and this solution even more worse)

@alexgarbarev
Copy link
Contributor

ok, let's see solution with subclass.

@alexgarbarev alexgarbarev reopened this Jul 29, 2016
alexgarbarev added a commit that referenced this issue Aug 26, 2016
…ubclass

Fixed the problem discussed in issues 513
@etolstoy
Copy link
Contributor

etolstoy commented Feb 5, 2017

Fixed in #524

@etolstoy etolstoy closed this as completed Feb 5, 2017
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

6 participants