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

Block element via CSS doesn't capture the correct element #904

Closed
srirambv opened this issue Aug 31, 2018 · 9 comments
Closed

Block element via CSS doesn't capture the correct element #904

srirambv opened this issue Aug 31, 2018 · 9 comments

Comments

@srirambv
Copy link
Contributor

Description

Block element via CSS doesn't capture the correct element

Steps to Reproduce

  1. Upgrade to 0.54.4
  2. Set VPN to EU to get the horrible IAB consent tool on the page
  3. Right click -> Brave->Block element via CSS on the IAB consent tool container, popup shows the element id. Click ok doesn't block the element. (see recording)

Actual result:

https://drive.google.com/open?id=1X6uPVaWtc59yGmDAaPSpX78HBz-yjUN2

Expected result:

Should correctly identify the element and block it

Reproduces how often:

100%

Brave version (about:brave info)

Brave 0.54.4 Chromium: 70.0.3528.4 (Official Build) dev (64-bit)
Revision 1911f781145b803e04f2d0b5b1a0146ae69fcbdc-refs/branch-heads/3528@{#9}
OS Windows

Reproducible on current release:

N/A

Website problems only:

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

Additional Information

cc: @Snuupy

@Snuupy
Copy link

Snuupy commented Sep 1, 2018

Thank you for adding this - it is a known issue (to me). The right click functionality currently uses the unique-selector library, which will need to be replaced with our own custom one.

@Crimix
Copy link

Crimix commented Sep 12, 2018

Hope that you do not only rely on id of element, because some website randomize the id. I have yet to get the CSS selector to block the correct element even when writing custom CSS selector

@srirambv
Copy link
Contributor Author

+1 from @rulatir via #3041

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

@rulatir
Copy link

rulatir commented Jan 22, 2019

+1 from @rulatir via #3041

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

Not cool. I did not +1 this issue, and I object to +1-ing it on my behalf by @srirambv who closed #3041 as a duplicate of this one without taking a minute to look into what I reported. #3041 has nothing to do with selector not matching the right element and everything to do with the display: none rule being overriden by the element's style attribute. No relation whatsoever to this issue. No relation whatsoever to the selector of the cosmetic filter rule.

OH WAIT, @srirambv is actually the reporter of this bug. He cannibalized my issue to bump his, abusing his collaborator privileges. Doubly not cool.

@srirambv
Copy link
Contributor Author

He cannibalized my issue to bump his, abusing his collaborator privileges

Ok you really got me with that statement. 😂

Reason for adding it here is not because I logged the issue, this issue is for tracking a feature of custom rules that can be added for the block element that would eventually be implemented. There is also an issue for adding a preview (#1280) before blocking. So this issue and #1280 would ideally have the same fix when implemented which would cover your usecase as well.

🖖

@rulatir
Copy link

rulatir commented Jan 22, 2019

You should have provided that explanation when closing my issue.

@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@Brave-Matt
Copy link

@chrisgagne
Copy link

Is there a workaround for this? E.g., some text file, etc, that I could edit that would allow the same end result?

@bsclifton
Copy link
Member

Closing as wontfix - we have a new cosmetic filter in place. I created #8914 to track removing this older version

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

No branches or pull requests

8 participants