-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YouTube] Extract trends from A/B tested "Videos" tab and fix extraction of trends name from A/B tested new title design #1045
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except the following comments, your changes look good to me.
However, I cannot reproduce the A/B test anymore. Do you think YouTube reverted their test or stopped it? This may lead to the removal of this parameter, and may break trends extraction instead of fixing it in the future if using these parameters return a bad response (unlikely to happen, as wrong parameters seem to be ignored).
.../java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeTrendingExtractor.java
Outdated
Show resolved
Hide resolved
.../java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeTrendingExtractor.java
Outdated
Show resolved
Hide resolved
I cannot get the A/B test with the videos tab any more, too. Maybe YouTube did scrap this idea, maybe it's just the end of the test run and they are evaluating the results. I would keep the change for now, if the new layout does not appear again within half a year we could take it out again for the sake of simplicity. Requesting non-existant tabs will simply return the default tab (it's the same thing for channel tabs), so I dont see any issues. |
are you using this PR or the regular version of the Extractor? Edit: oh, the error is about the title ("Trending") not being extracted. So it looks like a new header layout or something like that. Btw, why do we throw an exception when we fail to extract a text that is always the same? Edit2: Here we go: they changed the header renderer. Should be an easy fix. |
For localization purposes I think. When support of YouTube extraction in other content languages than English will be added again, an incorrect information would be returned for these non-English languages. |
I see. Falling back to "Trending" if the title could not be extracted would not be bad either, so I changed it. When and why was the extraction of other languages removed, though? |
35f0ee4
to
3673d4a
Compare
That's a very bad idea to return an English-only string, due to the reasons explained above: if YouTube changes their Trending title and how it is accessed, the extractor would return a value which would always pass the test. That's why I reverted this change.
It was removed in #158, due to parsing issues, as several parts of the YouTube extractor relies on English strings (age-restricted and paid content checks, accessibility data parsing, ...). I worked a few months ago on trying to restoring this support and I will try to resume my work after we have finished with major features on the extractor. |
YouTube is A/B testing a new layout for the trending page, where they moved the list of trending videos into a separate tab.
Issue reported on Reddit: https://www.reddit.com/r/NewPipe/comments/127tyh8/no_v%C3%ADdeos_is_it_a_bug/
Pictures here: https://code.thetadev.de/ThetaDev/rustypipe/src/branch/main/notes/AB_Tests.md#4-video-tab-on-the-trending-page
I have changed the extractor to fetch this new tab. If the new layout is not enabled, YT simple returns the Now page like before.
The parser had to be adjusted, because we cannot filter out all shelves with titles any more (the shelves in the Videos tab have titles).
Fixes #1046.