-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
NullReference fix in ShellPageRendererTracker.UpdateTabBarVisibility() #26786
Conversation
Hey there @Marioo1357! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
@PureWeen is this small change just hiding some sinister bug somewhere or is it expected that VC and Page may be null at this point? I see a few null checks in places, so I am not super afraid, but it is bugging me a bit. Regardless, this change will stop a crash, but the possible nulls are causing me to think things. |
It looks like the VC is GC-ing while this is still attached. I think that's not reason to block this PR as this PR should help somewhat. Though I think this might just push the exception down to UpdateLeftToolbarItems which also references the VC. It'd be good to have a followup PR where we check for VC going null in a couple more places. |
@mattleibow @PureWeen |
Ultimately yes, but just haven't scheduled this effort yet We'll look at doing a sweep through on NET10 we can't change any of the nullability on public APIs during a service release so we fix these in waves as we have time during major releases. I did what I could for now here which should help
Can you create an issue? |
Description of Change
Check if Page and ViewController are not null in ShellPageRendererTracker.UpdateTabBarVisibility()
Issues Fixed
Fixes #26784