FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Video title filter / blacklist

Open kommunarr opened this issue 10 months ago • 34 comments

Video & playlist title filter / blacklist

Pull Request Type

  • [ ] Bugfix
  • [x] Feature Implementation
  • [ ] Documentation
  • [ ] Other

Related issue

closes #1070 closes #3086 closes #95

Description

Enter a fragment, word, or phrase (case insensitive) to hide all videos with titles containing it throughout all of FreeTube, except History and videos inside of playlists. Special exclusion text appears for community posts. ~~Nothing is hidden on the Channel route.~~ Now those videos do not appear on any view (reasoning: you don't want to see videos or playlists with blacklisted text in any part of your FreeTube browsing experience). Incidentally: to set a minimum character length of 3 for blacklisted inputs, this PR implements a minInputLength prop in ft-input and ft-input-tags.

Other notes:

  • Fixes "Play Next Video" playing hidden recommended videos (hidden either by this or Hide Channels)
  • Fixes hidden Video Recommendations taking up height

Screenshots

Screenshot_20231021_044819 Screenshot_20231021_045130 Screenshot_20231021_045248

Testing

  • Ensure videos whose titles contain blacklisted tags are never visible on any route
  • Ensure tags are case-insensitive and can be substrings of words
  • Try inputting tags of length < 3
  • Enable autoplay with a hidden video at the top of the recommended videos data, and ensure that only the next visible recommendation is autoplayed, not any hidden ones

Desktop

  • OS: OpenSUSE Tumbleweed
  • OS Version: 2023xxxx
  • FreeTube version: 0.19.1

kommunarr avatar Oct 21 '23 10:10 kommunarr

A bunch of issues regarding shorts/live videos were closed as a duplicate #1070

So we should also take that into account somehow

  • A way to block a shorts on all pages
  • A way to block a live streams on all pages
  • A way to block upcoming live streams on all pages

U could also just remove Closes behind issue number

Whoops, I'll remove the closing. Although to be fair, those other aspects aren't made clear in the original post.

kommunarr avatar Oct 21 '23 10:10 kommunarr

@efb4f5ff-1298-471a-8973-3d47447115dc With the current information provided by YouTube, it's impossible to hide shorts on all pages, the only way we know something is a short, is if it's on a shorts specific page.

absidue avatar Oct 21 '23 11:10 absidue

Would it be possible to hide live, premiere and upcoming streams

We can probably hide currently live stuff and upcoming stuff, we won't be able to differentiate between live stream or premiere and we also can't do any of those for RSS.

absidue avatar Oct 21 '23 11:10 absidue

Would it be possible to hide live, premiere and upcoming streams

From what I can discern, given that they have a title property on their data, I think they are being hidden appropriately if they contain a banned title, which I believe is what is being asked. Do you know of any videos I can use to test / how I can find videos matching these conditions easily?

kommunarr avatar Oct 21 '23 14:10 kommunarr

I think having this pull request just do the title filtering is fine.

absidue avatar Oct 21 '23 14:10 absidue

Yeah, that sounds good to me. If #1070 is serving a secondary purpose of improving the expansiveness of the existing Hide Upcoming Premieres & Hide Live Streams features, it certainly doesn't seem clear to me why that is the case, but it's not in scope for this PR IMO.

kommunarr avatar Oct 21 '23 14:10 kommunarr

I agree it doesnt have to be added in this PR

but

Should we even implement that if we cant differentiate them when RSS is enabled?

Feels like a half baked feature

If the answer to this is no then i guess we can still put closes behind issue nr

Seems like a separate issue should be created for it. If we have sub-issues that are not referenced in main issue titles or descriptions, it's hard for people to search for those & inevitably leads to dupes.

kommunarr avatar Oct 28 '23 16:10 kommunarr

Doesnt this close https://github.com/FreeTubeApp/FreeTube/issues/3086 ?

Why we have so many dupes

Closes https://github.com/FreeTubeApp/FreeTube/issues/95

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Nov 09 '23 04:11 github-actions[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Nov 09 '23 05:11 github-actions[bot]

Why we have so many dupes

Closes #95

It seems a few users might want this feature. I mean I was just about to open an issue asking for this exact thing until I found this PR so it's nice to see someone implemented the thing :D @jasonhenriquez Nice job!

There is only one thing I would like to ask: It is possible to add a way to export the "blocklist" as a text file? You know as a backup file. That would be nice in the case you had a lot of blocked words.

gelatinbomb avatar Nov 16 '23 02:11 gelatinbomb

@gelatinbomb That will be possible once #1018 is implemented - a user-friendly means of sharing settings.db files. Until then, you can make a backup of the settings.db file on your device and grab it from there.

kommunarr avatar Nov 16 '23 04:11 kommunarr

  • Copy original video title from subscriptions -> Put it in the blacklist -> check subscription page -> video gone -> Enable DeArrow -> video still gone even though DeArrow title is different

    • Enable DeArrow -> Go to subscription page -> copy a word/title of a vid that has a title change -> insert into blacklist -> go to subscription page -> See video not gone

Implementation-wise, the irksome part of that is that the DeArrow logic is in the ft-list-video, but our video hiding logic bits have been in the video list container classes a level or two above that, which I believe is done that way for performance & logical reasons, correct me if I'm wrong (if you can't show the child, you can't show the parent wrapper, so that introduces problems that can't be solved easily by performance).

Behavior-wise, the main question is what would we want the expected behavior to be in both of these cases. Would you really want to see a video if either the DeArrow title or the original contained a banned word? I would think not, so that would imply that the most preferred behavior would be to filter if either matched. But then again, that does put a lot of power in DeArrow contributors, so maybe we shouldn't care about it to that extent. So I'm currently leaning on just ignoring the DeArrow title consistently, which is what the code is currently doing. Let me know what you think.

* Should we even do `word fragments`? What if i really hate `ores` for some reason so i insert `ore` but i really love `cores`, now i dont get to see all the beautiful vids about `cores` :(

I do think this is a worthwhile hypothetical. It's a trail that often ends in regex, which I think is probably a bit of overkill for the initial feature & isn't always good for usability (How do you indicate that this is a regex tag versus just a regular tag? If all tags are regex tags, will non - power users like it if they can't use special characters like they want? How do you handle errors? How does it affect performance?). So I think we should let it simmer and see how it goes for people.

On if you want to be able to differentiate it without regex, it becomes a question of "is the banning of partial words banning too much stuff for me" versus "is the banning of only full words banning too little". It's a good question to ask. I think the second one is closer to where people are, but I can't really know that for sure. Is it okay if my answer to this is "let's see how many people complain" and leave it there for now?

* Why not hide playlist titles and video titles within a playlist?

Good question! I was going off of what we were doing in our components, as discussed in the Matrix chat. I figure that the same push to add it to playlists will also be adding the channel logic as well. Unsure of the considerations there, and this PR is a little big, so I left it be. But I can add this and/or channel filtering logic to there as well if desired.

Edit: following the discussion in the Matrix chat, I'll address this.

kommunarr avatar Nov 17 '23 14:11 kommunarr

Regex support would be great. BlockTube supports it like this:

// all-caps
/[A-Z ]{15,}/
// all lowercase
/^[a-z ]{30,}/
// indian characters
/[\u0900-\u097F]+/
// french characters
/[àâçéèêëîïôûùüÿñæœ]+/

It's very useful when YouTube decides to only recommend foreign-language videos for some unknown reason and to block videos with all-caps clickbait titles.

Since regexes can be quite cryptic, it would be nice to have an optional comment field for each entry.

But that might indeed better be done in a separate PR.

Alexander-Wilms avatar Nov 17 '23 18:11 Alexander-Wilms

  • Copy original video title from subscriptions -> Put it in the blacklist -> check subscription page -> video gone -> Enable DeArrow -> video still gone even though DeArrow title is different

    • Enable DeArrow -> Go to subscription page -> copy a word/title of a vid that has a title change -> insert into blacklist -> go to subscription page -> See video not gone

Implementation-wise, the irksome part of that is that the DeArrow logic is in the ft-list-video, but our video hiding logic bits have been in the video list container classes a level or two above that, which I believe is done that way for performance & logical reasons, correct me if I'm wrong (if you can't show the child, you can't show the parent wrapper, so that introduces problems that can't be solved easily by performance).

Behavior-wise, the main question is what would we want the expected behavior to be in both of these cases. Would you really want to see a video if either the DeArrow title or the original contained a banned word? I would think not, so that would imply that the most preferred behavior would be to filter if either matched. But then again, that does put a lot of power in DeArrow contributors, so maybe we shouldn't care about it to that extent. So I'm currently leaning on just ignoring the DeArrow title consistently, which is what the code is currently doing. Let me know what you think.

Hmm i think when someone explicitly toggles DeArrow titles to show up they dont want to see click bait titles and want a title that tells them what the contents of the video will look like. If that title contains a word that is in the blacklist i think it should hide it. I also think it shouldnt hide a video with a replaced DeArrow title if the original title contains the blacklisted word

* Should we even do `word fragments`? What if i really hate `ores` for some reason so i insert `ore` but i really love `cores`, now i dont get to see all the beautiful vids about `cores` :(

I do think this is a worthwhile hypothetical. It's a trail that often ends in regex, which I think is probably a bit of overkill for the initial feature & isn't always good for usability (How do you indicate that this is a regex tag versus just a regular tag? If all tags are regex tags, will non - power users like it if they can't use special characters like they want? How do you handle errors? How does it affect performance?). So I think we should let it simmer and see how it goes for people.

On if you want to be able to differentiate it without regex, it becomes a question of "is the banning of partial words banning too much stuff for me" versus "is the banning of only full words banning too little". It's a good question to ask. I think the second one is closer to where people are, but I can't really know that for sure. Is it okay if my answer to this is "let's see how many people complain" and leave it there for now?

I agree lets let this simmer and come back to it when people open issues or start asking questions.

* Why not hide playlist titles and video titles within a playlist?

Good question! I was going off of what we were doing in our components, as discussed in the Matrix chat. I figure that the same push to add it to playlists will also be adding the channel logic as well. Unsure of the considerations there, and this PR is a little big, so I left it be. But I can add this and/or channel filtering logic to there as well if desired.

Will test soon!

I'll look into this. Out of curiosity, how is this problem handled with blocked channels?

Edit: It looks like they don't try to deal with that, which is understandable. There seem to be some sufficiently distinct UX issues with adding it to playlists that we may want to handle separately.

kommunarr avatar Nov 20 '23 23:11 kommunarr

Pre-existing issue carried over: when autoplay is enabled and a blocked channel's video (using existing channel block feature) is next up in the pre-filtered recommended videos, that video is loaded up next. Same issue occurs now with tan upcoming recommended video that was filtered due to its title. Almost certain that the same occurs for playlist autoplay as well.

kommunarr avatar Nov 21 '23 00:11 kommunarr

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Nov 21 '23 02:11 github-actions[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Nov 21 '23 02:11 github-actions[bot]

Thank for pointing that out, @efb4f5ff-1298-471a-8973-3d47447115dc - this feature is actually no longer supposed to work on History and videos inside of playlists, and I just had to fix the logic for not using this feature on certain routes. We can minimize that second part from inside playlists to just user playlists, but I'm thinking we may want to do that as a separate PR / effort where we enable both hidden channel & text preferences for playlists.

kommunarr avatar Nov 22 '23 00:11 kommunarr

Video is hidden in community post but creator was nice enough to include the url in the description of the post. Should we maybe hide the whole post to prevent someone from accessing the content they blacklisted?

They can if they want to, which seems fine to me.

kommunarr avatar Nov 22 '23 17:11 kommunarr

Hmm thought of an usecase that we maybe can consider in the decision of hiding it completely or not:

Having a child so the parents puts something in the blacklist -> password protect the settings because dont want them to change the blacklist -> child goes to community post but sees that vid is hidden but in the description there is a url to the video

The child could also have ez access to a browser to retrieve that url from the community post and drop it into FT so idk if this is even worth considering. I know this might be a stretch but i just wanted to throw it in here.

Hmm thought of an usecase that we maybe can consider in the decision of hiding it completely or not:

Having a child so the parents puts something in the blacklist -> password protect the settings because dont want them to change the blacklist -> child goes to community post but sees that vid is hidden but in the description there is a url to the video

The child could also have ez access to a browser to retrieve that url from the community post and drop it into FT so idk if this is even worth considering. I know this might be a stretch but i just wanted to throw it in here.

The part that I'm stuck at is that this isn't a YouTube feature to have those links present; this "duplicate link" is something some authors put in by choice on some of their videos. So we're kinda just adding logic to check all of the different text / links of a community post with a matching ID of a blocked video (if they shared the non vanity link) in the same post just because some authors put a duplicate video link there. It would need a more compelling use case to check for such an informal case, IMO.

Screenshot_20231123_065749

kommunarr avatar Nov 23 '23 13:11 kommunarr

All seems fine but one UX issue: When I test Try inputting tags of length < 3 There is no response from app when text < 2 chars input and enter pressed which makes it look broken Better to have a toast

PikachuEXE avatar Nov 25 '23 01:11 PikachuEXE

Weirdly am seeing this error when trying to add showToast into ft-input; have never seen anything like this before. Edit: just moved all the logic to ft-input-tags to handle this oddity.

renderer.js:15792 Uncaught ReferenceError: Cannot access '__WEBPACK_DEFAULT_EXPORT__' before initialization
    at Module.default (ft-input.vue:3:42)
    at eval (SubscribedChannels.js:15:1)
    at ./node_modules/babel-loader/lib/index.js!./src/renderer/views/SubscribedChannels/SubscribedChannels.js?vue&type=script&lang=js& (renderer.js:2863:1)
    at __webpack_require__ (renderer.js:15789:33)
    at fn (renderer.js:15998:21)
    at eval (SubscribedChannels.js?vue&type=script&lang=js&:5:144)
    at ./src/renderer/views/SubscribedChannels/SubscribedChannels.js?vue&type=script&lang=js& (renderer.js:5980:1)
    at __webpack_require__ (renderer.js:15789:33)
    at fn (renderer.js:15998:21)
    at eval (SubscribedChannels.vue:6:105)

kommunarr avatar Nov 25 '23 03:11 kommunarr

I am not sure if clearing input when input already exists is a good UX Maybe user wants to correct the string (make it more specific) It's easier to delete the whole input text then to retype the whole input text

PikachuEXE avatar Nov 26 '23 00:11 PikachuEXE