wordpress-mobile / WordPress-iOS

WordPress for iOS - Official repository
http://ios.wordpress.org/
GNU General Public License v2.0
3.7k stars 1.12k forks source link

URLSession feature flag leads to broken video uploads on self-hosted sites #22659

Closed SiobhyB closed 9 months ago

SiobhyB commented 9 months ago

Expected behavior

Videos uploaded to self-hosted sites should function as expected.

Actual behavior

With non-production builds, videos are currently being uploaded with an invalid format to self-hosted sites. This happened following the change in https://github.com/wordpress-mobile/WordPress-iOS/pull/22540, which defaulted to using URLSession in WordPressKit (instead of Alamofire).

An error like the following can be seen when attempting to view iOS-uploaded videos in the media library on the web:

Media error: Format(s) not supported or source(s) not found[Download](https://example.com.mp4?_=4)

Additionally, when uploaded to the editor, the videos won't play back.

Note, this isn't an issue on dotcom or Jetpack-connected sites, only self-hosted sites with no Jetpack connection.

Steps to reproduce the behavior

The feature flag that enables URLSession in WordPressKit can be toggled off via MeApp SettingsFeature Flags (note, I found it necessary to reboot the app for these changes to take effect). Toggling the flag off, however, doesn't fix the issue and leads to failed video uploads.

Tested on iPhone XR, iOS 17.3.1, Jetpack iOS / WordPress iOS non-production builds only
dangermattic commented 9 months ago

Thanks for reporting! 👍

SiobhyB commented 9 months ago

@crazytonyli, I wanted to surface this issue for you. Would you be able to take a look? With the current state, it's tricky to manually test whether changes to the video block work as expected for self-hosted sites, as all uploads are failing for non-production builds.

crazytonyli commented 9 months ago

@SiobhyB Thanks for reporting the issue! I have created a fix in https://github.com/wordpress-mobile/WordPressKit-iOS/pull/734. I'll update the app once the PR is merged.