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 filter - use !important for display property #3041

Closed
rulatir opened this issue Jan 22, 2019 · 3 comments · Fixed by brave/brave-core#3121
Closed

Cosmetic filter - use !important for display property #3041

rulatir opened this issue Jan 22, 2019 · 3 comments · Fixed by brave/brave-core#3121

Comments

@rulatir
Copy link

rulatir commented Jan 22, 2019

Test plan

See brave/brave-core#3121

Description

The cosmetic filters don't work with elements that set the display property using the style attribute.

Proposed solution: in the rule created by the cosmetic filter, the display property should be !important.

Please implement!

[Detailed STR on PR]

@srirambv
Copy link
Contributor

Probably covered under #904

@rulatir
Copy link
Author

rulatir commented Jan 22, 2019

Please give it a minute of your time to actually look into the issue. The element in question has an id attribute, so the style rule injected by the cosmetic filter is super simple:

#element-id {
   display: none;
}

But the element has style attribute with display: block, so this overrides display: none in the injected stylesheet. It's pure CSS as CSS behaves. Nothing to analyse further. This has nothing to do with wrong element being matched. Replacing the unique-selector library will not fix this. No tweaks to the selector of the cosmetic filter's rule will fix this. The element being matched is the right one, it's just that the cosmetic filter's style rule is too weak.

@NejcZdovc NejcZdovc reopened this Jan 28, 2019
@NejcZdovc NejcZdovc added this to the 1.x Backlog milestone Jan 28, 2019
@rebron rebron removed this from the 1.x Backlog milestone Feb 7, 2019
@rebron rebron added this to Untriaged / Incoming in Shields via automation Feb 12, 2019
@tildelowengrimm tildelowengrimm added the priority/P5 Not scheduled. Don't anticipate work on this any time soon. label Feb 15, 2019
@tildelowengrimm tildelowengrimm moved this from Untriaged / Incoming to Shields Bugs in Shields Feb 15, 2019
@bsclifton bsclifton added this to To do in Deprecated Cosmetic Filter via automation May 10, 2019
@bsclifton bsclifton removed this from Shields Bugs in Shields May 10, 2019
@Snuupy Snuupy moved this from To do to In progress in Deprecated Cosmetic Filter May 10, 2019
@Snuupy Snuupy moved this from Development to Testing in Deprecated Cosmetic Filter Jul 19, 2019
@Snuupy Snuupy moved this from Testing to Code Review in Deprecated Cosmetic Filter Jul 24, 2019
@petemill petemill added this to the 0.70.x - Nightly milestone Aug 9, 2019
Deprecated Cosmetic Filter automation moved this from Code Review to Done Aug 9, 2019
@btlechowski
Copy link

btlechowski commented Sep 26, 2019

Verification passed on

Brave 0.69.129 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#3121
For reddit portion I had to lower shields first to show promoted posts.

Verification passed on

Brave 0.69.129 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Verified passed with

Brave 0.69.129 Chromium: 77.0.3865.90 (Official Build) (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS macOS Version 10.13.6 (Build 17G5019)

@rebron rebron removed this from Done in Deprecated Cosmetic Filter Oct 1, 2019
@rebron rebron changed the title Cosmetic filter !important Cosmetic filter - use !important for display property Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment