[iOS] Prevent crash when a leaked cell has MenuItem with bindings #3288
Conversation
…nuItem with bindings and framework like Xamarin.Forms sets BindingContext to null after leaving the page
| @@ -632,6 +632,9 @@ UIView SetupButtons(nfloat width, nfloat height) | |||
|
|
|||
| void SetupSelection(UITableView table) | |||
| { | |||
| if (table.GestureRecognizers == null) | |||
| return; | |||
There was a problem hiding this comment.
I don't think we want to return here, or we won't run _tableView.AddGestureRecognizer(new SelectGestureRecognizer());
There was a problem hiding this comment.
It looks like when GestureRecognizers is null then there is no point in calling _tableView.AddGestureRecognizer(new SelectGestureRecognizer());, otherwise we would always had NRE. What I am trying to say is that when GestureRecognizers is null - that screen is gone and disposed.
There was a problem hiding this comment.
I'm not sure table and _tableView are the same reference here, though.
There was a problem hiding this comment.
You may be right, though. Just discussing. :)
There was a problem hiding this comment.
It is the same reference, SetupSelection is private and called only once, right after setting _tableView to table, easy to verify :)
There was a problem hiding this comment.
We can change _tableView to table and make the method static, nano-optimization =)
There was a problem hiding this comment.
It would certainly make it more clear. I'm on board with that approach!
There was a problem hiding this comment.
I have updated PR
|
This looks good. If you're able to add a test case to the Control Gallery, too, that would be excellent. Thank you! |
samhouts
added
the
approved
Description of Change
Prevent NRE and crash when a leaked cell in ListView with Recycle mode has MenuItem with bindings and framework like Prism.Forms sets BindingContext to null after leaving the page. Leaked cells can appear as result of device orientation change plus scrolling or just ListView.ScrollTo call.
Issues Resolved
API Changes
N/A
Platforms Affected
Behavioral/Visual Changes
N/A
PR Checklist