Regex error when parsing the Youtube JavaScript code #785
Conversation
nonword characters
|
I was not able to test it due to problems getting it to run with my local build of NewPipe. However, others have said it works. |
|
@AbduAmeen The tests are failing, can you look into that?
Did the guide from the readme not work? https://github.com/TeamNewPipe/NewPipeExtractor#testing-changes |
|
I was able to get the local build to work. The initial fix also needed to remove the braces. The fix provided here worked. |
This comment has been minimized.
This comment has been minimized.
|
@AbduAmeen Thanks so much for the fix! Could you please upload the APK so that we can link it in the pinned issue? |
|
This apk file will not install on my phone at all... Android 8 |
|
@AbduAmeen Could you please check that before we link it? |
|
@infinitewaveparticle it's a debug apk file, normal installation won't work. Try using adb install -t apkfile.apk |
|
@TechGuyOnTGB All our PR APK are debug ones and they install normally. I can confirm this one isn't installing. |
|
Sorry about that. I didn't realize android forces you to sign your apks. |
|
It's quite stupid that Android doesn't just tell you "this APK is unsigned so it can't be installed." |
|
@AbduAmeen yes the APK works as expected, tested inside Anbox. Hope it gets merged with the upstream code. Thank you for the effort you've put in! ^_^ |
| int arrayStartBrace = functionName.indexOf("["); | ||
|
|
||
| if (arrayStartBrace > 0) { |
There was a problem hiding this comment.
Are both paths of the if case covered by tests? Because I don't quite understand why this is needed and cannot be adjusted in the regex
There was a problem hiding this comment.
The condition is always true for the tests we have currently
There was a problem hiding this comment.
I tested a few more video ids and the condition was still true. However, I do not see a reason to remove the condition. I'd keep it as it is.
There was a problem hiding this comment.
The branching doesn't depend on video, but on the player version. To test the other case, you have to force an older player like https://www.youtube.com/s/player/8040e515/player_ias.vflset/en_US/base.js
|
Also, please use |
|
Any idea when the new apk will make it to F-Droid ? |
|
@Gardakant if you have newpipe's repo on f-droid, I think it should be released shortly. If you got newpipe from fdroid's default repo, expect to wait a bit, since it takes time to pull the source. |
|
please any one here help me i cant make change on this scripte YoutubeThrottlingDecrypter.java who can show me to find this java scripte and how i change it bcz i write to read only not possible to change on it |
|
When I try to install this version on Android 11, it looks like it's installing, then I receive a message that says, "App not installed". |
|
@jlhumbert Off-topic and cannot reproduce - but try installing with |
|
Okay, I will try that. Thanks!
…On Fri, Feb 11, 2022, 11:51 AM Grimoire ***@***.***> wrote:
@jlhumbert <https://github.com/jlhumbert> Off-topic and cannot reproduce
- but try installing with adb if you have a computer nearby, and look at
the (way more informative) error message.
—
Reply to this email directly, view it on GitHub
<#785 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJFOOKDJHPC2ORB5PU2LLKDU2U5C5ANCNFSM5NIIPCIQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
there is already a fixed version released. No need to install a debug version |
Changed the regex to account for nonword characters
Fixes: TeamNewPipe/NewPipe#7734