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

[Desktop] Add Block trackers & ads (aggressive) setting to apply cosmetic filtering to 1p items #8475

Closed
pes10k opened this issue Feb 29, 2020 · 4 comments · Fixed by brave/brave-core#5481
Assignees
Labels

Comments

@pes10k
Copy link
Contributor

pes10k commented Feb 29, 2020

Description

by default we have cosmetic filtering that attempts to differentiate 1p and 3p ads, and only block the latter.

Some users would like to do the simpler, "apply all cosmetic filters, always" approach. Basically, just do what uBO does.

This issue would be to modify the current cosmetic filtering to have an option to automatically inject all cosmetic filters, and not do party checks. '

Design

Shields panel
cosmetic filtering

brave://settings
cosmetic filtering shields settings

Figma for CSS, layout, sizing: https://www.figma.com/file/3A6F6VrxVahiZFxLr7j7FO/Desktop-Brave-Shields?node-id=16%3A14877

@pes10k pes10k added feature/shields The overall Shields feature in Brave. feature/shields/adblock Blocking ads & trackers with Shields priority/P3 The next thing for us to work on. It'll ride the trains. features/shields/cosmetic-filtering labels Feb 29, 2020
@pes10k
Copy link
Contributor Author

pes10k commented Feb 29, 2020

The implementation could be pretty simple:

  1. Add a content / per-site shields setting for cosmetic filtering (off, only 3p {default}, all)
  2. add a code path in the cosmetic filtering content script that, when the above is in the "all" mode, injects all matching rules (not just 3p ones)

@BrendanEich
Copy link
Member

I like even more minimal yet viable MVPs. Per https://twitter.com/myphs/status/1233110060903075844?s=20 what many want is the "uBO me globaly please" setting. That would be a single setting, could even start with brave:flags or better internal URL and no official settings UX. But what would it flip internally?

@karenkliu
Copy link

Designs for brave://settings added and dropdown option typo fixed.

@btlechowski
Copy link

btlechowski commented Jul 7, 2020

Verification passed on

Brave 1.11.89 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
Revision 8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#5481
Verified default setting
image
Verified default setting in brave://settings/shields
image

Verified aggressive setting in Brave Shields Panel works:
image
Verified aggressive setting in brave://settings/shields works:
image

Verified allowing all trackers and ads in Brave Shields works as expected
image

Verified allowing all trackers and ads in brave://settings/shields works as expected
Verified page is reloaded when changing the setting in Brave Shields.
Verified that the setting in brave://settings/shields is not updated by changes to the Shields panel


Verified passed using

Brave	1.11.90 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
Revision	8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS	macOS Version 10.14.6 (Build 18G3020)

Screen Shot 2020-07-07 at 5 12 30 PM

Std - shields inherit from settings

  • Set global Trackers & ads blocking to be Aggressive. Confirmed shields inherited this value. Confirmed left tile was green, right tile was green.

aggressive - shields inherit from settings

  • Set global Trackers & ads blocking to be Disabled. Confirmed shields inherited this value. Confirmed left tile was red, right tile was red.

disabled - shields inherit from settings

  • Left global Trackers & ads blocking to be Standard (default). Changed per site value to be Trackers & ads blocked (aggressive). Confirmed left tile was green, right tile was green. Confirmed global setting remained Standard.

site specific shield settings do not modify global - aggressive

  • Left global Trackers & ads blocking to be Standard (default). Changed per site value to be Allow all trackers & ads. Confirmed left tile was red, right tile was red. Confirmed global setting remained Standard.

site specific shield settings do not modify global - disabled

  • Set global Trackers & ads blocking to be Aggressive. Changed per site value to be Trackers & ads blocked (standard). Confirmed left tile was green, right tile was red. Confirmed global setting remained Aggressive.

site specific shield settings do not modify global - std


Verification passed on


Brave | 1.11.92 Chromium: 84.0.4147.68 (Official Build) dev (64-bit)
-- | --
Revision | e7b7234037639b407e69c5428c891ce018cd6c0d-refs/branch-heads/4147@{#772}
OS | Windows 10 OS Version 1903 (Build 18362.900)

Verified default setting in brave://settings/shields
image

Default site shield settings
image

When sites shield settings are set to Trackers & ads blocked (standard) left tile is displayed in Green and Right tile is displayed in Red
image

When sites shield settings are set to Trackers & ads blocked (aggressive) left tile and right tiles are displayed in Green
image

When site shield settings are set to Allow all trackers & ads left and right tiles are displayed in Red
image

When global shield setting for Trackers & ads blocked is set to Standard left tile is displayed in Green and Right tile is displayed in Red

image

When global shield setting for Trackers & ads blocked is set to Aggressive left and right tiles are displayed in Green
image

When global shield setting for Trackers & ads blocked is set to Disable left and right tiles are displayed in Red
image

@rebron rebron changed the title Allow cosmetic filtering to also be applied to 1p items [Desktop] Add Block trackers & ads (aggressive) setting to apply cosmetic filtering to 1p items Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants