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 cosmetic filters #13070

Closed
SergeyZhukovsky opened this issue Dec 7, 2020 · 7 comments · Fixed by brave/brave-core#6735
Closed

Implement cosmetic filters #13070

SergeyZhukovsky opened this issue Dec 7, 2020 · 7 comments · Fixed by brave/brave-core#6735

Comments

@SergeyZhukovsky
Copy link
Member

SergeyZhukovsky commented Dec 7, 2020

Description

We need a common solution for Android and desktop for that feature. The goal is to make it happen without using extensions.

Steps to Reproduce

  1. Go to chicagotribune.com
  2. Observe an empty space on top of the webpage
    tempFileForShare_20201207-104118

Actual result:

Expected result:

No empty space
Screenshot from 2020-12-07 10-43-18

Reproduces how often:

Desktop Brave version:

Android Device details:

  • Install type (ARM, x86):
  • Device type (Phone, Tablet, Phablet):
  • Android version:

Version/Channel Information:

  • Can you reproduce this issue with the current release?
  • Can you reproduce this issue with the beta channel?
  • Can you reproduce this issue with the nightly channel?

Other Additional Information:

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

Miscellaneous Information:

@LaurenWags
Copy link
Member

@SergeyZhukovsky is there a test plan for this other than the STR?

@SergeyZhukovsky
Copy link
Member Author

@pes10k @antonok-edm do we have any test plan for that from the original implementation?

@pes10k
Copy link
Contributor

pes10k commented Feb 15, 2021

@antonok-edm will know more automated tests than me, but I have been snowballing manual / QA tests here: https://dev-pages.brave.software/cosmetic-filtering/text-ads.html

Happy to add more if it'd be helpful

@antonok-edm
Copy link
Collaborator

@SergeyZhukovsky The original implementation was tracked at #5381. There was a test plan there, although as far as I can see, the website in question has since been updated so that it no longer has the ad spot.

@LaurenWags the STR here is a pretty good test case though 😄

@LaurenWags
Copy link
Member

perfect, thanks all! just wanted to make sure QA tested this appropriately 😄

@LaurenWags
Copy link
Member

LaurenWags commented Feb 15, 2021

Verified passed with

Brave | 1.21.54 Chromium: 88.0.4324.152 (Official Build) beta (x86_64)
-- | --
Revision | 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS | macOS Version 10.15.7 (Build 19H512)

Verified STR from description. Confirmed no empty space at the top:
chicagotribune

Confirmed "Trackers & ads blocked" setting values on https://dev-pages.brave.software/cosmetic-filtering/text-ads.html:

Aggressive Standard (default) Allow all
Aggressive Standard-default Allow All

Verification passed on

Brave 1.21.56 Chromium: 88.0.4324.152 (Official Build) dev (64-bit)
Revision 6579930fc53b4dc589c042bec9d0a3778326974d-refs/branch-heads/4324@{#2106}
OS Ubuntu 18.04 LTS

Verified STR from description. Confirmed no empty space at the top:
image

Verified "Trackers & ads blocked" setting values on https://dev-pages.brave.software/cosmetic-filtering/text-ads.html:

Aggressive Standard (default) Allow all
image image image

Verified passed with

Brave	1.21.59 Chromium: 88.0.4324.182 (Official Build) beta (64-bit)
Revision	73ee5087001dcef33047c4ed650471b225dd8caf-refs/branch-heads/4324@{#2202}
OS	Windows 10 OS Version 1909 (Build 18363.1256)

Verified STR from description. Confirmed no empty space at the top:
ChicagoTribune

Confirmed "Trackers & ads blocked" setting values on https://dev-pages.brave.software/cosmetic-filtering/text-ads.html:

Aggressive Standard (default) Allow all
Aggressive Standard Allow All

@srirambv
Copy link
Contributor

srirambv commented Mar 1, 2021

Verification passed on OnePlus 6T with Android 10 running 1.21.70 x64 build

Shields ON Shields Off
image image

Verification passed on OnePlus 6T with Android 10 running 1.21.70 x64 build

Shields ON Shields Off
image image

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