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

Implement per-site strict-blocking #2036

Closed
11 tasks done
dhowe opened this issue Feb 3, 2022 · 29 comments
Closed
11 tasks done

Implement per-site strict-blocking #2036

dhowe opened this issue Feb 3, 2022 · 29 comments

Comments

@dhowe
Copy link
Owner

dhowe commented Feb 3, 2022

There are several parts to this:

  • UI design for menu (see Update design for menu to clarify and support strict-blocking #2035)
  • Mechanism to add current site to StrictBlockList (remove it from whitelist when strictBlock selected and viceversa)
  • Mechanism for including strict-block-sites (should these go in adnauseam.txt?)
  • Remove allowAnyBlockOnDomains from code, and move (2) sites to default list
  • Remove calls and definition for isBlockableDomain() in core.js
  • Verify that default rules can be overridden by a user-rules in 'My Rules' panel
  • Verify that these show correctly in the logger
  • Add tests for all of these in release-testing (including these)
  • Add new FAQ entry(s)
  • When page is strictBlocked, have some kind of notification
  • Add new text entries to i18n file

See also: global strict-blocking

@mneunomne
Copy link
Collaborator

I still think that the per-website mechanism should be the same machanism as the whitelist one. Some reasons:

  • It would then have exact same behavior as when we disable a website on the menu (but instead of going to a whitelist, it would go to a "strictBlockList"
  • it would be easy to maintain thought a github based list
  • very clear to the user where it comes from (as with whitelist page)
  • easy to implement, same mechanism as whitelist
  • editable by the user

@dhowe
Copy link
Owner Author

dhowe commented Feb 3, 2022

Three options for where domain-specific rules (user-specific & default) can live:

  1. Include both default and user strict-blocks in 'My-Rules'
  2. Include both user and default strict-blocks in a new settings panel
  3. Include user strict-blocks in 'My-Rules', defaults in adnauseam.txt

Pros/cons:

  1. Easy to implement, perhaps unclear why default rules are there
  2. Clear, but requires new panel in settings, perhaps unclear why default rules are there, plus requires a new list with defaults somewhere?
  3. Perhaps the clearest, but difficult to implement as adnauseam.txt is a filter-list and would require added parsing, plus unclear how to override default rules

@mneunomne
Copy link
Collaborator

If we implenet this list as a "strictBlockList" like the whitelist work right now, I think MyRules can stay exactly the same. In regards to strictBlock it would simply be a more advanced way of using the strictBlock option, because it also permits the user to choose for script, images etc when making a new rule.

@mneunomne
Copy link
Collaborator

@dhowe I was trying to find a way to have the default rules as a file in the assets.json but how the assets are dealt with in the code is so tricky... Also we would need to put in a lot of thought how live-updates would relate to used-changeable rules.

I would say for now, keep as hard-coded as you suggested at some point.

@dhowe
Copy link
Owner Author

dhowe commented Feb 9, 2022

I think the easiest way would be to add a section in adnauseam.txt, but this is not trivial, so I'm ok with leaving it as is for now, at least until we have a longer list of sites

@mneunomne
Copy link
Collaborator

  • Verify that these show correctly in the logger

@dhowe the defaultStrictBlock domains now will go to "My Rules" Do we really need a specific Logging for them?

The moment they go to "My Rules" they are just together with all other Rules, for them to be specifically logged it would require some extra "reverse lookup". So not sure we need this spefic logging.

@dhowe
Copy link
Owner Author

dhowe commented Feb 14, 2022

ok as is

@mneunomne
Copy link
Collaborator

Regarding the discussion on which method to do we use implement this feature.

There are currently two possible approaches:

Using the Dynamic Rules to enforce the strict-blocks.

Pros:

  1. Easier to implement (no need to parse new lists, just need to add and remove the rules from "my rules" segment.
  2. ? (Hard for me to think another except that it is easier)

Cons:

  1. Harder to explain to users. (For example what exactly triggers the "strict" button to be on, only the following rule domain.com * * strictBlock ?
  2. Dynamic Blocking rules already have their own interface from uBlock
  3. Not consistent with the "pause button" which ads the domain to a white list.
  4. Hard to imagine how we would handle this if we one day want to decide to move this list to be updated online.

Creating a new "Strict-list" that does the opposite of what the "whitelist" does.

Pros:

  1. Consistent (exact opposite) with the "pause" button on the menu.
  2. Easier to explain to the users
  3. Better if we eventually implement a live update of the file coming from github

Cons:

  1. Harder to implement, would be a new file to be cached and parsed.
  2. Have to implement all the import export functionalities in it as well.

Regarding the dynamic rules I have some other concerns as well. I wonder what will happen when in a particular version we change the defaultStrickBlockList, but the user has also done that. Will the changes merge? Will the user prevail?

It seems that to implement on "My Rules" sounds easier at first but i fear that in the future it will be harder to maintain.

@dhowe please feel free to add your points as well.

@dhowe
Copy link
Owner Author

dhowe commented Feb 22, 2022

Seems that changes required to enable live updates are similar for both cases:

  • a set of rules (probably a section of adnauseam.txt) containing default-strict-blocks
  • the difference is where they would be inserted after parsing, either as dynamic rules or added to the allowAnyBlockOnDomains list, or ... (?)
  • same problem in both cases regarding your last point above, where a user may have removed a default entry from either the dynamic rules, or the strict-block list -- how to know if it had never been there or had been removed by user?

@mneunomne
Copy link
Collaborator

@dhowe yes true. The difference is that the dynamic blocking rules have extra parameters that might confuse the user (and complicate a tiny bit the code).

I would just insist in the "strictBlockList" because it sounds much more straightfoward to me.

dhowe added a commit that referenced this issue Feb 26, 2022
@mneunomne mneunomne mentioned this issue Mar 10, 2022
dhowe added a commit that referenced this issue Mar 11, 2022
@mneunomne mneunomne mentioned this issue Mar 17, 2022
dhowe added a commit that referenced this issue Mar 17, 2022
@mneunomne
Copy link
Collaborator

mneunomne commented Mar 18, 2022

Btw, based on the discussion on #1966, should we disable clicking for Strict Blocked websites? Or will that be a separate thing?

We can also do a checkbox in the Strict Block List for example, enabled/disabled the clicking for strict-blocked pages. This would potentially be an alternative solution for #1982

@dhowe
Copy link
Owner Author

dhowe commented Mar 19, 2022

my sense is that strict-blocking should not affect clicking at all -- the idea is to sacrifice some percentage of potential hide/clicks in order to block as much as possible, but not to counteract the primary functionality

@mneunomne
Copy link
Collaborator

@dhowe There is a minor issue with this design. uBlock doesn't have a function to do a reverse lookup if the filtering on that page is currently disabled because of a Domain or Page exception. Which means I can't check on JavaScript and make the user see which of these two options was previously enabled:

Screenshot 2022-03-24 at 14 04 05

To know if a page is disabled uBlock only has a Boolean, it doesn't tell us the 'cause' for the filtering to be disabled.

Can we maybe keep as it is in uBlock, where the control key changes the scope of the disabled (and also strict block)?

I think to make this change we would have to make a considerable amount of changes in the code, and it is perhaps unnecessary.

@mneunomne
Copy link
Collaborator

And @dhowe another question. Should we remove the global strict block functionality or not?

@dhowe
Copy link
Owner Author

dhowe commented Mar 24, 2022

I think this makes sense for domain vs page level blocking (as in ublock) - simply showing that the addon is disabled on the page

But I we probably need to show something different for the strict blocking case. Can we not just compare the list of domains that are strict-blocked to the current domain?

Also, I suppose that we need an order of operations for blocked vs strict-blocked; that is, what to do if both are activated...

@mneunomne
Copy link
Collaborator

But I we probably need to show something different for the strict blocking case.

I'm not sure we do. I would say that strict-blocking or disabled a specific page instead of a domain is really a fringe case. At least I haven't come across a case where blocking a specific page of a domain was really necessary.

Can we not just compare the list of domains that are strict-blocked to the current domain?

In the coding level it is a bit more complicated than that, there are like more 5 layers of functions that separate the actual list from the popupData that we check in order to make the state in the menu. It's doable I'm just wondering about the necessity of it. Also wondering if adding this level of information is actually necessary.

@mneunomne
Copy link
Collaborator

mneunomne commented Mar 24, 2022

And regarding the Global Strict Block...

If we want that functionality we could move the checkbox to the "Strict Block List" page instead, and when activated, add an alert on the menu with the option to disable it.

if we don't need it (as already discussed I believe), then we just remove it...

@dhowe
Copy link
Owner Author

dhowe commented Mar 24, 2022

I am not suggesting a page-based strict-block (we've already decided against this). I am only saying that we need to show the strict-block alert when the user is on a page within a strict-blocked domain.

In terms of global strict-block, I would suggest leaving the functionality in place for now, and seeing if a use-case arises. If so we can then add a UI option in settings (not in the menu). If not, we can remove it later...

dhowe added a commit that referenced this issue Mar 26, 2022
working dropdown on menu footer
dhowe added a commit that referenced this issue Apr 1, 2022
@mneunomne
Copy link
Collaborator

mneunomne commented Apr 2, 2022

Added the follwing to the test wiki:

    Menu Toggle Button

  • Active -> Adnauseam works normally. If the current page and/or domain can't be found both in "Whitelist" and "Strict Block List", then the toggle button should be on "active" state.
  • Strict -> Current domain is strict blocked. When the button is clicked, the DOMAIN is added to the "Strict Block List". If the toggle button was previously on "disabled", that page and/or domain is removed from the "Whitelist".
  • Disabled -> AdNauseam is disabled for the current domain/page. When the button is clicked, by default the DOMAIN is added to the "Whitelist". ALSO when clicked, a popup will appear with the options "on domain.com" and "on this page". Whenever a page/domain is added to the Whitelist, it will be automatically removed from the "Strict Block List" if it can be found there.
    • on domain -> the current DOMAIN (e.g. google.com) will be added to the "Whitelist", if the PAGE was previously on the "Whitelist", it will be then removed.
    • on this page -> the current PAGE (e.g. google.com/foo/bar.html) will be added to the "Whitelist", if the DOMAIN was previously on the "Whitelist", it will be then removed.

Strict Block List

  • Add a domain name into the list and check if a page from that domain is following the strict block behaviour.
    Add a page path into the list and check if only that specific page from the domain is following the strict block behaviour
  • Make sure when changing the state of the domain from the Popup menu, the Strict Block List is responding accordingly.
  • Make sure when list is edited directly, the popup menu follows the behaviour correctly.

@mneunomne
Copy link
Collaborator

mneunomne commented Apr 2, 2022

  • when the released is finished we need to review the whole test page for outdated info

@dhowe
Copy link
Owner Author

dhowe commented Apr 2, 2022

couple of small edits above

@mneunomne
Copy link
Collaborator

mneunomne commented Apr 2, 2022

couple of small edits above

updated

dhowe added a commit that referenced this issue Apr 3, 2022
@mneunomne
Copy link
Collaborator

It is working now, need to merge it together with the 1.40.0 merge and see test so we can make the new release.

We can leave merge 1.41.X for the next release.

mneunomne added a commit to mneunomne/AdNauseam that referenced this issue Apr 15, 2022
@mneunomne
Copy link
Collaborator

Sent the PR with the feature to our master branch together with the 1.40.4 merge

#2079

@mneunomne
Copy link
Collaborator

Implementing the red icon when the page is strictBlocked:

#2080

Screenshot 2022-04-18 at 13 28 02

@dhowe
Copy link
Owner Author

dhowe commented Apr 18, 2022

looks great -- lets make sure to add the '-' for 'strict-blocked' and also consistently align the question mark (vertically)...

@mneunomne
Copy link
Collaborator

Implemented on v3.13.1, closing

@mneunomne
Copy link
Collaborator

@dhowe I realised that I have made a conceptual mistake when implementing this feature.

I made it so, that when a domain is strict-blocked Adnauseam is strict-blocking all the requests coming from that domain, but not the third-party requests coming from other domains.... A bit dumb from my part to be honest. I am sending a PR now that fixes this.

@mneunomne mneunomne reopened this May 22, 2022
@mneunomne mneunomne mentioned this issue May 22, 2022
dhowe added a commit that referenced this issue May 23, 2022
@mneunomne
Copy link
Collaborator

Fixed, closing again

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

No branches or pull requests

2 participants