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

cosmetic blocking flag causes shields ad toggle to not work correctly #8688

Open
LaurenWags opened this issue Mar 13, 2020 · 7 comments
Open

cosmetic blocking flag causes shields ad toggle to not work correctly #8688

LaurenWags opened this issue Mar 13, 2020 · 7 comments

Comments

@LaurenWags
Copy link
Collaborator

@LaurenWags LaurenWags commented Mar 13, 2020

Description

If you enable the cosmetic blocking flag (you must do this manually on 1.5.x, but it is enabled by default on 1.7.x), and you change the cross site trackers toggle to OFF, ads do not start appearing on web pages as expected. You must toggle shields off entirely.

Note - if you disable cosmetic blocking flag, this is not an issue, the Cross-site trackers toggle works as expected.

Steps to Reproduce

  1. If using 1.5.x, enable the brave://flags/#brave-adblock-cosmetic-filtering flag. If using 1.7.x it should be enabled by default.
  2. Navigate to a site like slashdot.org --> ads not displayed as expected.

Default shields - cosmetic blocking flag enabled

  1. Open shields, toggle off Cross-site trackers blocked.
    --> ads are not displayed on the page

ad block toggled off - cosmetic blocking enabled

  1. Toggle shields off entirely.
    --> now ads are displayed on the page

shields off entirely - cosmetic blocking enabled

If you disable the brave://flags/#brave-adblock-cosmetic-filtering, then the Cross-site trackers blocked toggle begins working as expected (after browser restart.

Actual result:

When cosmetic blocking flag is enabled and you toggle Cross-site trackers blocked OFF, sites do not show ads as expected

Expected result:

Toggling Cross site trackers blocked to OFF should show ads regardless of the cosmetic blocking flag setting:
ad block toggled off - cosmetic blocking disabled

Reproduces how often:

easily

Brave version (brave://version info)

Brave 1.5.111 Chromium: 80.0.3987.132 (Official Build) (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS macOS Version 10.14.6 (Build 18G3020)
Brave 1.7.58 Chromium: 80.0.3987.132 (Official Build) dev (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS macOS Version 10.14.6 (Build 18G3020)

Version/Channel Information:

  • Can you reproduce this issue with the current release? 1.4.x n/a, but yes to 1.5.x
  • Can you reproduce this issue with the beta channel? yes
  • Can you reproduce this issue with the dev channel? yes
  • Can you reproduce this issue with the nightly channel? unsure but probably

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? disabling shields entirely is the solution
  • Does the issue resolve itself when disabling Brave Rewards?
  • Is the issue reproducible on the latest version of Chrome?

Miscellaneous Information:

reproduces on other sites like buzzfeed.com, etc.

@LaurenWags
Copy link
Collaborator Author

@LaurenWags LaurenWags commented Mar 13, 2020

for comparison, here's how the same site behaves when cosmetic blocking flag is Disabled on brave://flags

Default shields:
Default shields - cosmetic blocking disabled

Only Cross-site trackers blocked toggled off:
ad block toggled off - cosmetic blocking disabled

Shields off entirely:
shields off entirely - cosmetic blocking disabled

@LaurenWags
Copy link
Collaborator Author

@LaurenWags LaurenWags commented Mar 13, 2020

cc @rebron @kjozwiak @bsclifton as this will be an issue in 1.5.x, but the flag must be enabled manually on 1.5.x. we should fix this for 1.7.x release though.

@pes10k
Copy link
Contributor

@pes10k pes10k commented Mar 13, 2020

@LaurenWags is the expected behavior here to tie cosmetic filtering to the Cross-site trackers toggle? If so, i'd vote against that, since there is (in almost all cases) no connection between the cosmetic filtering and tracking.

Would either of the following solutions work?

  1. keeping the flag for cosmetic filtering, enabled by default, but which people can turn off globally if they really don't want it
  2. add a shields toggle for cosmetic filtering

FWIW, i have a weak pref against putting cosmetic filtering in shields, since the shield metaphor suggests privacy-protections

@LaurenWags
Copy link
Collaborator Author

@LaurenWags LaurenWags commented Mar 13, 2020

@pes10k those are probably better aimed at product for a preferred solution 😄 I was just confused when I toggled cross-site trackers (the "ad" toggle) and ads didn't display - until I tracked it down to the cosmetic blocking flag.

@rebron could you take a look @pes10k suggestions?

@ryanbr
Copy link
Collaborator

@ryanbr ryanbr commented Mar 14, 2020

Having a separate shields switch for cosmetic filtering would be helpful for debugging purposes if there an issue with a cosmetic filter we could track down more quickly

@pes10k
Copy link
Contributor

@pes10k pes10k commented Mar 16, 2020

I've got no beef with another toggle, its just a matter of slotting it in w/ the other priorities. I know @tomlowenthal is also pushing for more shield configurability. @ryanbr could you create an issue for it, if it doesn't exist here, and we can triage and work through it from there?

@rebron rebron added this to Untriaged Backlog in General Mar 16, 2020
@rebron rebron removed this from Untriaged / Incoming in Shields Mar 17, 2020
@rebron rebron added the priority/P3 label Mar 17, 2020
@rebron
Copy link
Collaborator

@rebron rebron commented Mar 17, 2020

@rebron rebron added priority/P2 and removed priority/P3 labels Mar 24, 2020
@rebron rebron moved this from Untriaged Backlog to In progress in General Mar 24, 2020
@rebron rebron moved this from In progress to P1 & P2 Backlog in General Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
General
  
P1 & P2 Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.