Crash With Older YouTube Versions (18.x)

[joshuaseltzer]

First off, thanks for putting this tweak together, it's pretty fantastic.

I have some older devices and I find that older versions of YouTube work better on those. Specifically I'm using 18.34.5. Your tweak compiles and installs just fine, but the latest version is crashing when I click on the Share button. Here's the relevant crash log:

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[YTELMContext fromView]: unrecognized selector sent to instance 0x280a47e20'

It looks like fromView was added with this commit: https://github.com/jkhsjdhjs/youtube-native-share/tree/17ad96645297b6fc9656ae12de67ae6fc69fa192

If I find some time to try understand the code and what exactly it's doing, I'll try to fix this one myself but I wanted to bring this to your awareness in the meantime.

[jkhsjdhjs]

Alright, thanks for reporting this. Can you test this commit? bff7555

[joshuaseltzer]

Sure - I'll test it out later today when I have a chance to pull in those changes and deploy on my device.

Probably unrelated: I was also testing with some earlier 19.x versions (specifically 19.20.2) and on that version it didn't crash, but it also did not show the native share sheet - instead the original share sheet was showing. I can investigate more with this version later today too if that would be helpful.

[jkhsjdhjs]

Sure, feel free to investigate it. Displaying the original share sheet is the fallback behavior if something goes wrong (the return %orig in the following snippet):

youtube-native-share/Tweak.x

Lines 164 to 181 in 6ad6798

	%hook ELMPBShowActionSheetCommand
	- (void)executeWithCommandContext:(ELMCommandContext*)context handler:(id)_handler {
	if (!self.hasOnAppear)
	return %orig;
	GPBExtensionDescriptor *innertubeCommandDescriptor = [%c(YTIInnertubeCommandExtensionRoot) innertubeCommand];
	if (![self.onAppear hasExtension:innertubeCommandDescriptor])
	return %orig;
	YTICommand *innertubeCommand = [self.onAppear getExtension:innertubeCommandDescriptor];
	GPBExtensionDescriptor *updateShareSheetCommandDescriptor = [%c(YTIUpdateShareSheetCommand) updateShareSheetCommand];
	if(![innertubeCommand hasExtension:updateShareSheetCommandDescriptor])
	return %orig;
	YTIUpdateShareSheetCommand *updateShareSheetCommand = [innertubeCommand getExtension:updateShareSheetCommandDescriptor];
	if (!updateShareSheetCommand.hasSerializedShareEntity)
	return %orig;
	if (!showNativeShareSheet(updateShareSheetCommand.serializedShareEntity, context.context.fromView))
	return %orig;
	}
	%end

You could insert several debug log statements to see how far it gets.

[joshuaseltzer]

Sure, feel free to investigate it. Displaying the original share sheet is the fallback behavior if something goes wrong (the return %orig in the following snippet):

youtube-native-share/Tweak.x

Lines 164 to 181 in 6ad6798

	%hook ELMPBShowActionSheetCommand
	- (void)executeWithCommandContext:(ELMCommandContext*)context handler:(id)_handler {
	if (!self.hasOnAppear)
	return %orig;
	GPBExtensionDescriptor *innertubeCommandDescriptor = [%c(YTIInnertubeCommandExtensionRoot) innertubeCommand];
	if (![self.onAppear hasExtension:innertubeCommandDescriptor])
	return %orig;
	YTICommand *innertubeCommand = [self.onAppear getExtension:innertubeCommandDescriptor];
	GPBExtensionDescriptor *updateShareSheetCommandDescriptor = [%c(YTIUpdateShareSheetCommand) updateShareSheetCommand];
	if(![innertubeCommand hasExtension:updateShareSheetCommandDescriptor])
	return %orig;
	YTIUpdateShareSheetCommand *updateShareSheetCommand = [innertubeCommand getExtension:updateShareSheetCommandDescriptor];
	if (!updateShareSheetCommand.hasSerializedShareEntity)
	return %orig;
	if (!showNativeShareSheet(updateShareSheetCommand.serializedShareEntity, context.context.fromView))
	return %orig;
	}
	%end

You could insert several debug log statements to see how far it gets.

Sorry for taking a while to get back to you on this - I've been pretty busy and haven't had a ton of free time to continue working on this. I finally managed to find some time this morning, and I can confirm that your prior commit prevented the crash. However, just like the 19.20.2 app, the share sheet wasn't showing.

The issue seems to be related to issue #7. When you changed the implementation with that issue, it broke compatibility with versions prior to YouTube 19.35.3 (according to the author of that issue, at least). I tried to reorganize the older code with the new implementation in order for it to work across all versions. I tested on my devices and it seems to work, but it might not be organized in the way you would like. Check out my branch (this commit, specifically) and let me know what you think.

I wasn't able to figure out how to get the /shorts/ URL path (from your commit here) with the older implementation so that it is one remaining thing that would need to be fixed.

[jkhsjdhjs]

Hmm, I see that you basically copied the old implementation and put it in alongside the current implementation. This leads to a lot of duplicate code unfortunately. Wouldn't it be possible to create a case-distinction solely for the parts changed in b4fd811?

Maybe by implementing two separate protobuf parser functions, one for the old implementation and another one for the new, that returns the same information? The function creating URL could be used in both cases then.

[joshuaseltzer]

Hmm, I see that you basically copied the old implementation and put it in alongside the current implementation. This leads to a lot of duplicate code unfortunately. Wouldn't it be possible to create a case-distinction solely for the parts changed in b4fd811?

Maybe by implementing two separate protobuf parser functions, one for the old implementation and another one for the new, that returns the same information? The function creating URL could be used in both cases then.

Yeah I agree. I'll be honest I wasn't super happy with how it turned out because it did seem like there was a lot of reused code / logic. I'll also admit that I did this knowing basically nothing about the protobuf API or code, so I wasn't exactly sure where else I could reduce the redundant code. That's why I definitely wasn't comfortable submitting a PR until I could spend some more time thinking it through. The other glaring issue is the protobuf code is marked as deprecated so we have to ignore deprecations too.

Going forward my free time to work on this will be limited so I'm not sure when I'll be able to understand it better to optimize the older code. At least we know that it's still possible for this tweak to work on the prior versions of YouTube though.

[jkhsjdhjs]

Yeah I agree. I'll be honest I wasn't super happy with how it turned out because it did seem like there was a lot of reused code / logic. I'll also admit that I did this knowing basically nothing about the protobuf API or code, so I wasn't exactly sure where else I could reduce the redundant code. That's why I definitely wasn't comfortable submitting a PR until I could spend some more time thinking it through.

Yeah, I also don't have a good idea on how to remove the redundant code, I guess we'd probably have to create a new container for the parsed protobuf data, where the old and new implementation can store their extracted data, as I'm not sure whether the old unknown fields API is compatible with the new one (probably not).

OR: If we could solve #7 somehow, we could use the same API with the new and old youtube versions. But I also don't have any idea on how to solve that one.

The other glaring issue is the protobuf code is marked as deprecated so we have to ignore deprecations too.

True, but I guess there's no way around that if we want to support older YouTube versions.

Going forward my free time to work on this will be limited so I'm not sure when I'll be able to understand it better to optimize the older code.

Yeah, same here. I'll leave this issue open for visibility, so people with older YouTube versions can just use your fork instead. If someone is motivated to work on this, I'll be happy to review a PR!

[joshuaseltzer]

Yeah I agree. I'll be honest I wasn't super happy with how it turned out because it did seem like there was a lot of reused code / logic. I'll also admit that I did this knowing basically nothing about the protobuf API or code, so I wasn't exactly sure where else I could reduce the redundant code. That's why I definitely wasn't comfortable submitting a PR until I could spend some more time thinking it through.

Yeah, I also don't have a good idea on how to remove the redundant code, I guess we'd probably have to create a new container for the parsed protobuf data, where the old and new implementation can store their extracted data, as I'm not sure whether the old unknown fields API is compatible with the new one (probably not).

OR: If we could solve #7 somehow, we could use the same API with the new and old youtube versions. But I also don't have any idea on how to solve that one.

The other glaring issue is the protobuf code is marked as deprecated so we have to ignore deprecations too.

True, but I guess there's no way around that if we want to support older YouTube versions.

Going forward my free time to work on this will be limited so I'm not sure when I'll be able to understand it better to optimize the older code.

Yeah, same here. I'll leave this issue open for visibility, so people with older YouTube versions can just use your fork instead. If someone is motivated to work on this, I'll be happy to review a PR!

I'm OK with people using my repo for legacy support. I can't guarantee I'll always be able to sync with your repo, but that shouldn't matter anyway if people are using is solely for older versions of YouTube.

Since the old code is deprecated and likely should not change, one option I had originally thought of was to create a new branch (maybe call it "legacy") and using that branch to include the legacy code which works with YouTube < 19.35.3. It could even use an older version of the protobuf API so that the older code is no longer marked as deprecated.

The only downside would be that users would need to know which package to install based on the version of YouTube they have. The solution used in my repo is able to work with all versions since we can do a check for the unknownFields property which indicates the legacy code should be used.

[jkhsjdhjs]

I'm OK with people using my repo for legacy support. I can't guarantee I'll always be able to sync with your repo, but that shouldn't matter anyway if people are using is solely for older versions of YouTube.

This tweak is basically "finished" anyway, not really much to change here. Also, this is just a free open-source project, we don't guarantee anything anyway, so no worries.

Since the old code is deprecated and likely should not change, one option I had originally thought of was to create a new branch (maybe call it "legacy") and using that branch to include the legacy code which works with YouTube < 19.35.3. It could even use an older version of the protobuf API so that the older code is no longer marked as deprecated.

It probably wouldn't change much, butt stuff like #8 should still be backported to the legacy branch. Also, I'd have to package two versions of the tweak. Best option for me would still be to somehow get rid of the redundant code and just merge it into main. Since the code would be shared between both implementations, the maintenance overhead would be minimal.

[joshuaseltzer]

I'm OK with people using my repo for legacy support. I can't guarantee I'll always be able to sync with your repo, but that shouldn't matter anyway if people are using is solely for older versions of YouTube.

This tweak is basically "finished" anyway, not really much to change here. Also, this is just a free open-source project, we don't guarantee anything anyway, so no worries.

Since the old code is deprecated and likely should not change, one option I had originally thought of was to create a new branch (maybe call it "legacy") and using that branch to include the legacy code which works with YouTube < 19.35.3. It could even use an older version of the protobuf API so that the older code is no longer marked as deprecated.

It probably wouldn't change much, butt stuff like #8 should still be backported to the legacy branch. Also, I'd have to package two versions of the tweak. Best option for me would still be to somehow get rid of the redundant code and just merge it into main. Since the code would be shared between both implementations, the maintenance overhead would be minimal.

Sounds like a good plan. Until then you can point people to my branch since I can confirm it works on older versions.

[jkhsjdhjs]

Yep, just added it to the readme 👍