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

Re-allow cosmetic filtering to override in-page !important styles after regression #22264

Closed
antonok-edm opened this issue Apr 12, 2022 · 6 comments · Fixed by brave/brave-core#13079

Comments

@antonok-edm
Copy link
Collaborator

brave/brave-core#12038 originally provided a fix for #20177, allowing cosmetic filters returned by the adblock engine to be injected into pages with a higher precedence than the page's stylesheets.

Unfortunately, the new method used to inject those CSS rules involved creating a new stylesheet for each rule, rather than the previous method which would simply add rules to a single stylesheet. That turned out to have a significant performance impact, especially for some users and websites; it was reverted in brave/brave-core#12950.

There doesn't appear to be a way exposed by Chromium to inject a privileged stylesheet from JS that also works on Android, or from C++ that doesn't involve creating an entire new stylesheet. Fixing this will likely involve patching StyleEngine to support a new stylesheet specifically used for cosmetic filtering.

@antonok-edm
Copy link
Collaborator Author

Note that #13079 fixed it for all cases except for general class/id injections in standard blocking mode. That will be solved in a followup and is tracked in #22474.

@LaurenWags
Copy link
Member

LaurenWags commented May 16, 2022

Verified with

Brave | 1.38.118 Chromium: 101.0.4951.67 (Official Build) (x86_64)
-- | --
Revision | 8888ee7a24e2c36661ddb9536c35b7d4852a3a98-refs/branch-heads/4951@{#1230}
OS | macOS Version 12.3.1 (Build 21E258)

Test Case #1 - #22030 (comment)

  • 1.38.115 Chromium: 101.0.4951.64 - without fix
  • 1.38.118 Chromium: 101.0.4951.67 - with fix

Shields enabled:

  • Using 1.38.115 Chromium: 101.0.4951.64 - 738s, 549s, 518s, 570s, 632s
  • Using 1.38.118 Chromium: 101.0.4951.67 - 580s, 489s, 483s, 484s, 572s
1.38.115 Chromium: 101.0.4951.64 Example 1.38.118 Chromium: 101.0.4951.67 Example
A C

Shields disabled:

  • Using 1.38.115 Chromium: 101.0.4951.64 - 445s, 465s, 470s, 450s, 475s
  • Using 1.38.118 Chromium: 101.0.4951.67 - 394s, 414s, 380s, 426s, 384s
1.38.115 Chromium: 101.0.4951.64 Example 1.38.118 Chromium: 101.0.4951.67 Example
B D

Test Case #2 -https://www.foodnetwork.com/recipes/food-network-kitchen/classic-shrimp-scampi-8849846

1.38.115 Chromium: 101.0.4951.64 Example 1.38.118 Chromium: 101.0.4951.67 Example
1 2

@MadhaviSeelam
Copy link

Verification PASSED with

Brave | 1.38.118 Chromium: 101.0.4951.67 (Official Build) (64-bit)
-- | --
Revision | 8888ee7a24e2c36661ddb9536c35b7d4852a3a98-refs/branch-heads/4951@{#1230}
OS | Windows 11 Version 21H2 (Build 22000.675)

Test Case #1 - #22030 (comment)

  • 1.38.115 Chromium: 101.0.4951.64 - without fix
  • 1.38.118 Chromium: 101.0.4951.67 - with fix

Shields enabled:
*Using 1.38.115 Chromium: 101.0.4951.64: 290ms, 276ms, 298ms. 288ms
*Using 1.38.118 Chromium: 101.0.4951.67: 237ms, 297ms, 287ms. 339ms

1.38.115 Chromium: 101.0.4951.64 1.38.118 Chromium: 101.0.4951.67
drun1 Run 1

Shields disabled:

*Using 1.38.115 Chromium: 101.0.4951.64 - 289ms, 455ms, 252ms, 275ms
*Using 1.38.118 Chromium: 101.0.4951.67 - 217ms, 250ms, 229ms, 287ms

1.38.115 Chromium: 101.0.4951.64 Example 1.38.118 Chromium: 101.0.4951.67
drun1 run1

Test Case #2 -https://www.foodnetwork.com/recipes/food-network-kitchen/classic-shrimp-scampi-8849846

1.38.115 Chromium: 101.0.4951.64 Example 1.38.118 Chromium: 101.0.4951.67 Example
Screenshot 2022-05-16 110655 ex1

@Uni-verse Uni-verse added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label May 16, 2022
@Uni-verse
Copy link
Contributor

Uni-verse commented May 17, 2022

Verification Completed using 1.38.118, Chromium 101.0.4951.67 on Samsung GS 21 running Android 12

Test Case 1: https://news.ycombinator.com/item?id=30932095

Shields ON:

1.38.118 : 572s, 702s, 668s, 630s, 504s
1.38.113: 690s, 633s, 596s, 567s, 628s

Shields OFF:

1.38.118: 474s, 622s, 664s, 576s, 404s
1.38.113: 455s, 594s, 487s, 468s, 472s

Test Case 2: https://www.foodnetwork.com/recipes/food-network-kitchen/classic-shrimp-scampi-8849846

1.38.118 1.38.113
Screen Shot 2022-05-16 at 7 33 56 PM Screen Shot 2022-05-16 at 7 38 49 PM

@Uni-verse
Copy link
Contributor

Uni-verse commented May 17, 2022

Verification Completed using 1.38.118, Chromium 101.0.4951.67 on Samsung Tab S7 running Android 12

Test Case 1: https://news.ycombinator.com/item?id=30932095

Shields ON:

1.38.118 : 731ms, 883ms, 789ms, 766ms, 847ms
1.38.113: 881ms, 755ms, 877ms, 922ms, 840ms

Shields OFF:

1.38.118: 678ms, 713ms, 681ms, 615ms, 627ms
1.38.113: 766ms, 799ms, 706ms, 952ms, 742ms

Test Case 2: https://www.foodnetwork.com/recipes/food-network-kitchen/classic-shrimp-scampi-8849846

1.38.118 1.38.113
Screen Shot 2022-05-16 at 8 16 09 PM Screen Shot 2022-05-16 at 8 12 22 PM

@btlechowski
Copy link

Verification passed on

Brave 1.38.119 Chromium: 101.0.4951.67 (Official Build) (64-bit)
Revision 8888ee7a24e2c36661ddb9536c35b7d4852a3a98-refs/branch-heads/4951@{#1230}
OS Ubuntu 18.04 LTS

Test Case 1: https://news.ycombinator.com/item?id=30932095

Shields ON:

1.38.119 : 4.92s, 3.99s, 4.82s, 4.51s, 4.75s
1.38.115: 4.05s, 5.14s, 4.44s, 4.89s, 4.61s

Shields OFF:

1.38.119: 6.05s, 5.41s, 5.89s, 5.38s, 6.01s
1.38.115: 5.42s, 5.24s, 5.59s, 5.55s, 6.10s

Test Case 2: https://www.foodnetwork.com/recipes/food-network-kitchen/classic-shrimp-scampi-8849846

1.38.119 1.38.115
image image

@LaurenWags LaurenWags removed the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment