corin8823 / Popover

Popover is a balloon library like Facebook app. It is written in pure swift.
MIT License
2.1k stars 327 forks source link

[Bug] resignFirstResponder races with .show(…), causing popover visual disappearance & effective memory leak + state corruption #86

Closed paulshapiro closed 7 years ago

paulshapiro commented 7 years ago

Hi corin8823,

One of our QA engineers discovered a bug I never saw in my own testing. He has it happen on all of his devices and I have it happen on none of mine! Imagine that. I debugged it over VNC.

Just before I present my popover, I seek the first responder and have it resign.

Then I instantiate my popover and hold a strong reference to it in the spawning button object.

As a result, we found a bug whereby the tooltip would apparently disappear immediately after presenting it, after spawning the popover while a text field in our form had first responder.

We traced it down by confirming that the view remained in the hierarchy after the bug occurred, yet was within a (probably private) keyboard variant of UIWindow instead.

I traced down the bug to the usage of UIApplication.shared.windows.last in the derivation of the so-called rootView in the three .show(…) codepaths. I believe what is occurring is that windows.last is indeed the keyboard input view at that time.

This causes a fatal condition in the app due to state sanity loss which can't be detected. In other words, whenever the user taps on the button, func tapped() is called, which immediately trips assert(self.popover == nil). The reason we make this assumption is the existence of the touch-intercepting curtainView within the popover which causes its dismissal.

For now I'm just patching the present-in-view UIView? derivation to UIApplication.shared.delegate!.window! within a common accessor (in the form of a derived property).

Thanks for your great work !! Paul

paulshapiro commented 7 years ago

Oops, I only now see you updated the code. Will try that out.