Skip to content

Check NRV visibility before modifying constraints#11550

Merged
ScoutHarris merged 2 commits intodevelopfrom
fix/11387-nrv_constraint_crash
Apr 25, 2019
Merged

Check NRV visibility before modifying constraints#11550
ScoutHarris merged 2 commits intodevelopfrom
fix/11387-nrv_constraint_crash

Conversation

@ScoutHarris
Copy link
Copy Markdown
Contributor

Fixes #11387

According to the Fabric logs, this crash seems to happen when searching the Free Photo Library.

51 | 1556091505912 | 🔵 Tracked: stock_media_accessed
52 | 1556091512547 | 🔵 Tracked: stock_media_searched
53 | 1556091514021 | 🔵 Tracked: stock_media_searched
54 | 1556091515163 | 🔵 Tracked: stock_media_searched
55 | 1556091515401 | 🔵 Tracked: stock_media_searched
56 | 1556091520549 | 🔵 Tracked: stock_media_searched
57 | 1556091525780 | 🔵 Tracked: stock_media_searched

I was not able to reproduce the crash, but this sort of crash typically happens when the NRV isn't actually visible so it fails when setting the constraints according to the view. So I added a check to ensure the NRV is visible before setting the constraints. 🤞 this fixes it.

To test:

  • Access the Free Photo Library.
  • Perform searches that have no results.
  • Verify it doesn't crash.

no_results

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@ScoutHarris ScoutHarris added this to the 12.4 milestone Apr 24, 2019
@ScoutHarris ScoutHarris requested a review from danielebogo April 24, 2019 22:31
@ScoutHarris ScoutHarris self-assigned this Apr 24, 2019
Copy link
Copy Markdown
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ScoutHarris for adding this! The code looks good and I never got a crash while I was testing it! :shipit:

@ScoutHarris ScoutHarris merged commit f725c9f into develop Apr 25, 2019
@ScoutHarris ScoutHarris deleted the fix/11387-nrv_constraint_crash branch April 25, 2019 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NoResultsViewController.configureTitleViewConstraints crash

2 participants