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

Enable CosmeticFilteringJsPerformance by default #26861

Closed
atuchin-m opened this issue Nov 18, 2022 · 10 comments · Fixed by brave/brave-core#15993
Closed

Enable CosmeticFilteringJsPerformance by default #26861

atuchin-m opened this issue Nov 18, 2022 · 10 comments · Fixed by brave/brave-core#15993

Comments

@atuchin-m
Copy link
Contributor

The feature was implemented in #25614
Cosmetic in child frames now is enabled by griffin in brave/brave-variations#466
Now the feature should be enabled with the recommended parameters.
Slack thread: https://bravesoftware.slack.com/archives/C01LKMP6X36/p1668103608663869

Steps to verify:

  1. Cosmetic filtering should works as before, nothing should be changed. For sites with a lot of DOM changes another algorithm is used, but the results should be the same.
  2. Child frames + Speedometer2 score: Enabling/disabling flag BraveAdblockCosmeticFilteringChildFrames shouldn't affect the score on https://browserbench.org/Speedometer2.0/. Pplease take into account that local results will have some deviations.
@LaurenWags
Copy link
Member

@atuchin-m

Can you please clarify

  1. Cosmetic filtering should works as before, nothing should be changed. For sites with a lot of DOM changes another algorithm is used, but the results should be the same.

from #26861 (comment)? Are there certain tests we should re-run, perhaps from a previous issue? Labelling as QA/Blocked until this is sorted.

cc @kjozwiak @rebron

@atuchin-m
Copy link
Contributor Author

@LaurenWags
Just to clarify: this PR only enable the fix from #26861.
The original fix changes the algorithm of detection of new elements that should be hidden by cosmetic filters.

As far as I concern we don't have a special test sites set. @pes10k @antonok-edm may be you know?

@atuchin-m
Copy link
Contributor Author

Also I suggest the following test:

  1. add this to brave://adblock custom filters:
##.blocked-class-1
##.blocked-class-2
##.blocked-class-3
  1. go to example.com and open dev tools console
  2. run this script that creates a bunch of blocked elemets:
function addElements(cls, num) {
  for (let i = 0; i < num; i++) {
    const newDiv = document.createElement("div");
    const newContent = document.createTextNode("Bad ad, should be blocked;" + cls);
    newDiv.appendChild(newContent);
    newDiv.classList.add(cls);
    document.body.appendChild(newDiv);
  }
  console.log('Added ', num, 'elements with class', cls);
}

function sleep(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

async function test() {
  addElements("blocked-class-1", 2000);

  await sleep(2000);
  addElements("blocked-class-2", 2000);

  await sleep(11000);
  addElements("blocked-class-3", 10);
  
  console.log('Done! Scroll to the end to check if there is any visible new element');
}
test()
  1. Check if you see added element at the end of the document.
  2. Remove added custom rules and repeat steps 2-4.

@LaurenWags
Copy link
Member

@atuchin-m so should the result from whether you have the custom rules in above case be the same? Not quite clear on what the expected result is for step 5 when we re-run the case from #26861 (comment) without the added rules.

@atuchin-m
Copy link
Contributor Author

The added elements simulate some unwanted ads and the custom rules simulate adblock files rules.

So if you have those custom rules them all the added element should be hidden.
If you don't have the rules: they should be visible.

@btlechowski btlechowski added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Dec 6, 2022
@btlechowski
Copy link

Verification passed on

Brave 1.47.115 Chromium: 108.0.5359.94 (Official Build) beta (64-bit)
Revision 713576b895246504ccc6b92c2fb8ce2d60194074-refs/branch-heads/5359_71@{#3}
OS Ubuntu 18.04 LTS

Cosmetic filtering not enabled - PASS

  1. open example.com
  2. Open dev tools
  3. run the script from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  4. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console

Verified the dynamically added blocks are not blocked
image
image
image

Cosmetic filtering enabled - PASS

  1. add this to brave://adblock custom filters:
##.blocked-class-1
##.blocked-class-2
##.blocked-class-3
  1. open example.com
  2. Open dev tools
  3. run the script from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  4. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console

Verified the dynamically added blocks are blocked
image
image

@btlechowski btlechowski added QA Pass-Linux and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Dec 6, 2022
@MadhaviSeelam
Copy link

MadhaviSeelam commented Dec 8, 2022

Verification PASSED

Brave | 1.47.115 Chromium: 108.0.5359.94 (Official Build) beta (64-bit)
-- | --
Revision | 713576b895246504ccc6b92c2fb8ce2d60194074-refs/branch-heads/5359_71@{#3}
OS | Windows 11 Version 21H2 (Build 22000.1219)

Cosmetic filtering not enabled - PASSED

  1. install 1.47.115
  2. launch Brave
  3. open example.com
  4. Open dev tools console
  5. run the script in step 3 from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  6. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console

Confirmed the dynamically added blocks are not blocked

ex1 ex2 ex3 ex4
image image image image

Cosmetic filtering enabled - PASSED

  1. continue from Case 1
  2. open brave://adblock and add following:
    ##.blocked-class-1
    ##.blocked-class-2
    ##.blocked-class-3
    
  3. refresh example.com site
  4. Open dev tools
  5. run the script in step 3 from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  6. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console

Confirmed dynamically added blocks are blocked

ex1 ex2
image image

Child frames + Speedometer2 score - PASSED

Verified as #26861 (comment)

  1. open brave://flags
  2. enable #brave-adblock-cosmetic-filtering-child-frames
  3. visit https://browserbench.org/Speedometer2.0/
  4. click Start Test

Confirmed little variation in the score when BraveAdblockCosmeticFilteringChildFrames flag enabled/disabled

enabled

ex1 ex2 ex3
image image image

disabled

  1. open brave://flags
  2. disable #brave-adblock-cosmetic-filtering-child-frames
  3. visit https://browserbench.org/Speedometer2.0/
  4. click Start Test
ex1 ex2 ex3
image image image

@stephendonner
Copy link

stephendonner commented Dec 11, 2022

Verified PASSED using

Brave 1.47.123 Chromium: 108.0.5359.99 (Official Build) beta (x86_64)
Revision 410951fc34bb4b2cbf182231f9f779efaafaf682-refs/branch-heads/5359_71@{#9}
OS macOS Version 13.1 (Build 22C65)

Cosmetic filtering not enabled - PASSED

  1. install 1.47.123
  2. launch Brave
  3. open example.com
  4. open dev tools console
  5. run the script in step 3 from https://github.com/brave/brave-browser/issues/26861#issuecomment-1335605869
  6. wait till Done! Scroll to the end to check if there is any visible new element is shown in the console appears

Confirmed dynamically added elements aren't blocked

example example example
Screenshot 2022-12-11 at 9 39 10 AM Screenshot 2022-12-11 at 9 37 45 AM Screenshot 2022-12-11 at 9 38 06 AM

Cosmetic filtering enabled - PASSED

continue from Case 1

  1. open brave://adblock and add following:
    ##.blocked-class-1
    ##.blocked-class-2
    ##.blocked-class-3
  2. refresh example.com site
  3. open dev tools
  4. run the script in step 3 from https://github.com/brave/brave-browser/issues/26861#issuecomment-1335605869
  5. wait till Done! Scroll to the end to check if there is any visible new element is shown in the console message appears

Confirmed dynamically added elements are blocked

example example
Screenshot 2022-12-11 at 9 20 01 AM Screenshot 2022-12-11 at 9 19 39 AM

Child frames + Speedometer2 score - PASSED

`Enabled`
  1. open brave://flags
  2. enable #brave-adblock-cosmetic-filtering-child-frames
  3. visit https://browserbench.org/Speedometer2.0/
  4. click Start Test
run 1 run 2 run 3
Screenshot 2022-12-11 at 9 49 02 AM Screenshot 2022-12-11 at 9 49 44 AM Screenshot 2022-12-11 at 9 50 25 AM

Disabled

  1. open brave://flags
  2. disable #brave-adblock-cosmetic-filtering-child-frames
  3. visit https://browserbench.org/Speedometer2.0/
  4. click Start Test
run 1 run 2 run 3
Screenshot 2022-12-11 at 9 53 42 AM Screenshot 2022-12-11 at 9 54 33 AM Screenshot 2022-12-11 at 9 55 15 AM

@GeetaSarvadnya GeetaSarvadnya added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jan 9, 2023
@GeetaSarvadnya GeetaSarvadnya removed the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jan 9, 2023
@Uni-verse
Copy link
Contributor

Verified on Samsung Galaxy Tab S7 using the following build(s):

Brave	1.47.168 Chromium: 109.0.5414.87 (Official Build) (64-bit) 
Revision	2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS	Android 12; Build/SP2A.220305.013
Cosmetic filtering enabled
  1. open example.com
  2. Open dev tools
  3. run the script from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  4. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console
  • Verified that dynamically generated elements are blocked
Example Example
screenshot-1673369814069 Screen Shot 2023-01-10 at 11 57 53 AM
Cosmetic filtering NOT enabled
  1. open example.com
  2. Open dev tools
  3. run the script from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  4. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console
  • Verified that dynamically generated elements are NOT blocked

Screen Shot 2023-01-10 at 11 50 39 AM
Screen Shot 2023-01-10 at 11 50 57 AM
Screen Shot 2023-01-10 at 11 51 16 AM

Speedometer Tests
#brave-adblock-cosmetic-filtering-child-frames Enabled #brave-adblock-cosmetic-filtering-child-frames Disabled
screenshot-1673370825899 screenshot-1673371077235

@Uni-verse
Copy link
Contributor

Uni-verse commented Jan 10, 2023

Verified on Samsung Galaxy S21 using the following build(s):

Brave	1.47.168 Chromium: 109.0.5414.87 (Official Build) (64-bit) 
Revision	2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS	Android 12; Build/SP2A.220305.013
Cosmetic filtering enabled
  1. open example.com
  2. Open dev tools
  3. run the script from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  4. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console
  • Verified that dynamically generated elements are blocked
Example Example
screenshot-1673371454630 Screen Shot 2023-01-10 at 12 25 17 PM
Cosmetic filtering NOT enabled
  1. open example.com
  2. Open dev tools
  3. run the script from Enable CosmeticFilteringJsPerformance by default #26861 (comment)
  4. Wait till Done! Scroll to the end to check if there is any visible new element is shown in the console
  • Verified that dynamically generated elements are NOT blocked

Screen Shot 2023-01-10 at 12 23 12 PM
Screen Shot 2023-01-10 at 12 23 26 PM
Screen Shot 2023-01-10 at 12 23 42 PM

Speedometer Tests
#brave-adblock-cosmetic-filtering-child-frames Enabled #brave-adblock-cosmetic-filtering-child-frames Disabled
screenshot-1673371774995 screenshot-1673371657090

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