Skip to content
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

Add a feature to block by site #184

Open
wingman-jr-addon opened this issue Jan 9, 2023 · 9 comments
Open

Add a feature to block by site #184

wingman-jr-addon opened this issue Jan 9, 2023 · 9 comments

Comments

@wingman-jr-addon
Copy link
Owner

The feature or similar has been suggested a number of times - I think either a whitelist/blacklist or as a button/refresh like uBlock Origin does would work well (or perhaps in conjunction).

@wingman-jr-addon
Copy link
Owner Author

(More feedback from the v3.3.3/3.3.4 upgrades)

@bmaupin
Copy link

bmaupin commented Nov 3, 2023

Hi, I'm interested in contributing to this feature. Do you have an idea of how you would want it to work?

For myself, I think in general the number of sites I have to interact with that I find problematic is relatively small, so just from my perspective I think I would probably benefit most from a blacklist. A whitelist would be acceptable but in order to benefit from stricter filtering I'd get a lot of false positives, and as a result I think I'd end up needing to add a much larger number of sites to a whitelist than I would a blacklist.

What are your thoughts?

Thanks!

@wingman-jr-addon
Copy link
Owner Author

Copying the comment we had over on #197 regarding how it currently works as a starting point:

Regarding blacklisting: yes, that is not implemented. I see you had visited the right issue #184 so I believe that's meant to be linked in your comment rather than #138 in case anybody else is looking. There is actually a static whitelist now over here:

wingman_jr/whitelist.js

Line 1 in d803502
function whtIsWhitelisted(url) {

With respect to PR's - yes, but something like this may get a bit interesting as it propagates out to the processor(s). There is also currently no UI or build framework and I'm quite stingy about dependencies as this works better for the submission process. If you have an idea in mind, let's maybe continue the discussion over on #184 before trying to work on something.

So at a quite basic level, it's as easy as extending the checks there - those functions already tie into all the right places for checking to occur so in some ways it's a guide.

There are a couple of key things to know when performing development here:

  1. Due to the reviewing procedure, I prefer to keep the number of external libraries as small as possible and to not implement a more complex build system.
  2. The overall system architecture is client server, with processor.js being launched from background.js. I didn't originally choose this but an issue with how GPU-related code was interacting in background.js forced me to basically rewrite the addon and shove the processor.js clients in hidden pages so that GPU interaction worked properly. Unfortunately of course this complicates development and enforces more strongly division between the two halves - in some ways good, in many ways harder.

Fortunately in this case we get lucky and it appears that all the logic for this portion is done "server"-side using direct calls from e.g. background.js.

So if we assume that this got implemented via the options somehow, the following are probably true:

  • whitelist.js needs to get the values from the browser.local.storage in some way. Note that some of the other settings are handled fully in background.js and use e.g. bkUpdateFromSettings. This helps handle reloading when options change, so that is a point to consider.
  • options.html/options.js will get updates to handle the UI part.

As far as the exact user experience, to me it sounds like your need would be satisfied by having a textbox with each line being e.g. a regex as a match, and something like that could work well with a future layer on top like the uBlock feature to turn the plugin on/off per site by just adding to the key list. So starting with a list would be a step towards that as well even though it wouldn't get all the way.

What do you think?

@bmaupin
Copy link

bmaupin commented Nov 6, 2023

That sounds great. I'll work on some sort of minimal implementation and send it to you for review before I get too far. Does that sound okay with you?

Due to the reviewing procedure, I prefer to keep the number of external libraries as small as possible and to not implement a more complex build system.

As an open-source maintainer myself, I can appreciate this :) I've found it worth putting things like this in a CONTRIBUTING.md file when projects start to get more than a few contributors. GitHub will link to it any time someone opens a new issue or pull request.

@wingman-jr-addon
Copy link
Owner Author

wingman-jr-addon commented Nov 7, 2023

Fair enough - I didn't actually start the project with the primary intent of getting any contributions, only as a way to open the source and get feedback about bugs. But given that you'll likely be the second or third contributor to source, it's probably time!
And yes, sounds like a good strategy regarding implementation. Please be aware I do not check GitHub every day so if I don't get back to you quite as quickly as you'd like that may be why.

@bmaupin
Copy link

bmaupin commented Nov 7, 2023

I just saw you responded regarding a whitelist but not a blacklist. Do you think it's feasible or desirable to create a blacklist?

Rather than block all images from a domain, in particular I was thinking domains in the blacklist would be automatically handled as if they were untrusted.

What do you think?

@wingman-jr-addon
Copy link
Owner Author

Ah, I just was indicating that the whitelist is what is already implemented (in a hardcoded way) so it was good to look at as a guide. Both whitelist and blacklist, so yes a blacklist would be desirable. What you're suggesting with treating as "untrusted" is a bit different, though - and I think a bit more technically complex, but useful.
In an ideal world, I think it could be like this: the filter list is a list of pairs, where each pair is a 1) a regex for matching the site URL and 2) a filter setting. The regexes would be checked in order and if a match was found, the filter setting would be used.
Filter settings would consist of the following:

  • whitelist - let the image through without checking, useful for things like captchas
  • blacklist - block the image without checking, useful for things like known bad sites
  • default - let the image be filtered by current zone setting, the usual setting
  • untrusted | normal | trusted - filter the image using the specified zone, useful if a certain site has a known profile, such as an image site being more suspect etc.

Implicitly, the last item of the list would be the regex matching everything with filter setting auto.

Now, how about implementation of the above?

  • whitelist - easy and can be checked fully server-side, already implemented in a basic way
  • blacklist - easy but you have to think what you want the blocking behavior to be - currently the processor provides the served SVG in case of blocking.
  • default - easy, what is already done
  • untrusted | normal | trusted - unfortunately a bit harder with current implementation, because right now the processors (clients) are notified of the threshold using bkNotifyThreshold. There are different ways one could handle this, with perhaps the easiest being to change to simply include the threshold to judge by on a per-image / per-video basis - but definitely some new plumbing.

So I think it might be best to start with something like the above filter list but initially only support whitelist, blacklist, and default. Then the others can be incrementally added later.

What do you think?

@bmaupin
Copy link

bmaupin commented Nov 10, 2023

I've been trying to chew on this for a bit. I don't think a blacklist that blocks all images is something I would use personally. For me the whole point of using this plugin is to take advantage of the AI to block images so I don't have to block all the images on a domain. Otherwise, I already use an ad blocker and those can easily block all images on a domain.

Having said that, if I would use a whitelist then it follows that someone might want a blacklist, and it makes more sense to keep that functionality in this extension rather than require somebody to use an ad blocker which is designed for a different purpose.

Beyond that, what would the purpose of having a "default" filter be, given that it would more or less be a no-op? Is it just a way to be explicit for certain domains?

Lastly, the UX feels like it might be a bit user-unfriendly. Requiring the users to type out "whitelist," "blacklist," etc. feels slightly cumbersome but also error-prone. But it's hard to think of a better solution:

  • I feel like the ideal solution would be a textbox next to a dropdown containing each of the options that could be selected. But this would also require add/delete buttons or some other way of managing entries and could get messy and complicated very quickly. Whereas your proposal could be a textbox which could simply scroll as it gets bigger.
  • I also thought you could have separate text boxes for each of the different filters. This might be the cleanest approach. For now it would only require 2-3 textboxes (whitelist/blacklist/default?) and as many as 6 total. This would be certainly easier for users but would potentially add clutter to the UI.

What do you think?

@wingman-jr-addon
Copy link
Owner Author

Earlier in the thread you'd mentioned:

For myself, I think in general the number of sites I have to interact with that I find problematic is relatively small, so just from my perspective I think I would probably benefit most from a blacklist.

In this comment you mentioned:

I don't think a blacklist that blocks all images is something I would use personally.

Those two seem contradictory. Might be worth explaining what you mean a bit more so I can make sure I'm understanding correctly.

With respect to UX, I think one thing to keep in mind is that we probably have generally two types of user: casual, and tech-savvy. I don't think the casual users would ever find going into options and adding something to a list - no matter how pretty or clear the UI is - to be user-friendly. It'd have to be out in front, like the way uBlock Origin has the on/off switch that turns on and off while you're on a specific page. That's what I think the most user-friendly approach is, or something close to it.

But - once you get to a point of entering a list, I think you could make it fancy with two boxes but for most tech-savvy users I know, they won't care and may actually prefer text. So I'd say keep that part as simple as possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants