Fix Crash on iOS when using MediaElement inside a CarouselView (again) #2245
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ne0rrmatrix
approved these changes
Oct 3, 2024
4 tasks
KeithBoynton
added a commit
to KeithBoynton/CommunityToolkitMaui
that referenced
this pull request
Oct 31, 2024
* main: Update dependabot.yml Tell Dependabot to ignore required libraries from auto-updating [housekeeping] Automated PR to fix formatting errors (CommunityToolkit#2295) [housekeeping] Automated PR to fix formatting errors (CommunityToolkit#2292) Fix animation behavior hot reload (CommunityToolkit#2288) Update resourceManagement.yml [housekeeping] Automated PR to fix formatting errors (CommunityToolkit#2285) [Housekeeping] Update NuGet Packages (CommunityToolkit#2254) Implement the ability to Close a popup (CommunityToolkit#1688) Add `[Obsolete]` tag Fix FileSaver Progress (CommunityToolkit#2277) Ensure `TouchBehavior.LongPressCompleted` event fires when `LongPressCommand is null` (CommunityToolkit#2239) Add Popup Support for MediaElement on iOS + MacCatalyst (CommunityToolkit#2251) [Sample app] Add MediaElementMultipleWindowsPage (CommunityToolkit#2250) [Sample App] Add Second CarouselView to `MediaElementCarouselViewPage.xaml` (CommunityToolkit#2248) Fix Windows Autoplay bug (CommunityToolkit#2238) Fix Crash on iOS when using MediaElement inside a CarouselView (again) (CommunityToolkit#2245) Refactor URLs and improve logging consistency (CommunityToolkit#2228) Bump xunit from 2.9.1 to 2.9.2 in /samples (CommunityToolkit#2234) Increase `CommunityToolkitSampleApp_Xcode_Version` to `16`
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
We initially fixed #1903 in PR #2187. However, it is back in iOS 18 (see screenshot).
The underlying bug is still the same: we need to add the
AVPlayerViewControllerused byMediaElementas a childUIViewControllerto theMauiMediaElementUIView.However, testing on iOS 18 shows that iOS has changed how it adds children to the UIViewControllerHierarchy, invalidating our previous work-around which was to wait for the DataTemplate to be initialized on the ItemsView event and then retrieve the
ItemsViewControllerfromItemsView.Handler.In this PR, we retrieve the
ItemsViewControllerby traversing the Descendants of the current visible page, accomplishing the same goal.Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
This new approach has two downsides:
ItemsViewcontrols, we are unable to determine whichItemsViewcontains ourMediaElementWindows, we are unable to determine which window contains ourMediaElementI believe both of these can be resolved with a bit more effort + time, and I've added a
TODOfor both scenarios in the code base.However, these two scenarios are rare; I estimate that the vast majority of our users will not encounter this scenario. Therefore, I'd like to merge this bug fix now, and we can add the unsupported scenarios in a future PR.
However, since these two scenarios are very rare, I'd like to merge this as a hot fix today and then we can focus on fixing the edge cases.