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

Bug: Filters containing classes with double dashes are ignored #1885

Open
danielyli opened this Issue Aug 9, 2016 · 14 comments

Comments

Projects
None yet
4 participants
@danielyli

danielyli commented Aug 9, 2016

Describe the issue

Custom filters that specify an element with a class name that contains a double dash (or double underscore) are ignored. This issue is reproducible for any page with an element whose class name contains a double dash or double underscore.

For example, say I want to block the top navigation bar that peeks in on Medium sites when you scroll the site up. My filter is:

##div.metabar--affixed

The filter is ignored.

(Use this Medium article as the test case.)

If I change the filter selector to specify a class without the double dash, the filter works.

Steps for anyone to reproduce the issue

  1. Add the filter ##div.metabar--affixed
  2. Navigate to https://medium.com/@ageitgey/machine-learning-is-fun-part-4-modern-face-recognition-with-deep-learning-c3cffc121d78.
  3. Observe that the filter is not applied (the top navigation bar still creeps in when scrolling up on the site).

Your settings

All settings are default.

  • Browser/version: Chrome 52.0.2743.116 (64-bit)
  • uBlock Origin version: uBlock Origin v1.8.4
Your filter lists

Default filter lists + the test case specified above

@gwarser

This comment has been minimized.

Contributor

gwarser commented Aug 9, 2016

Works when specific, not visible in log.

@gorhill

This comment has been minimized.

Owner

gorhill commented Aug 9, 2016

I can't find any element with the class .metabar--affixed on the test page. This is what dev console says:

> document.querySelector('.metabar--affixed');
< null
@gorhill

This comment has been minimized.

Owner

gorhill commented Aug 9, 2016

Never mind, got it while scrolling up, as you said.

@gorhill

This comment has been minimized.

Owner

gorhill commented Aug 9, 2016

It's not the double-dash, it's because the class .metabar--affixed is added/removed as one scroll up/down. uBO does not listen to class/id changes in its mutation observer.

In any case, this is not ideal as a cosmetic filter (because it is removed/added dynamically), .metabar--postArticle (or even better because specific filters are always better: medium.com##.metabar--postArticle) works better here, as it is a permanent class on the div.

Edit: Ok, I can see the benefit of using .metabar--affixed, it just prevents the top bar from floating while scrolling, while not getting rid of it completely. I have a fix locally, but I want to better evaluate its impact.

@danielyli

This comment has been minimized.

danielyli commented Aug 9, 2016

What's interesting is that on some other sites (e.g., this) uBlock accommodates the dynamic addition of classes:

  1. Navigate to this article. The header bar peeks in when scrolling up. With dev tools, we see that scrolled-up is one of the classes dynamically added to the element.
  2. Create filter: arstechnica.com##header.site-header.is_stuck.scrolled-up
  3. Refresh. The element no longer peeks in when scrolling up.
  4. To make a minimal example, change the filter to: arstechnica.com##header.scrolled-up
  5. Refresh. The element is blocked when you scroll up.
@gorhill

This comment has been minimized.

Owner

gorhill commented Aug 9, 2016

Because specific filters are used in your arstechnica.com example. Specific filters (ex. arstechnica.com##header.scrolled-up) are always injected, but generic ones (ex. ##header.scrolled-up) are injected only after uBO's DOM surveyor finds a match. Not watching for class/id changes prevent uBO from finding out whether new generic filters must be injected when a new class or id is merely added to an existing DOM element.

@danielyli

This comment has been minimized.

danielyli commented Aug 9, 2016

Ah, I see. Great, thanks for the explanation.

@ryanbr

This comment has been minimized.

ryanbr commented Dec 24, 2017

I've started to notice a trend of a certain CMS's using the "__" div elements. Given the age of this bug, is going to be fixed?

Was trying to filter on tunein.com "##.footer__socialLink___1Iby-" Even the following didn't work;

tunein.com##div[class*="socialLink"]
tunein.com##div[class^="footer__socialLink"]

@gorhill

This comment has been minimized.

Owner

gorhill commented Dec 24, 2017

The bug title is incorrect, it has nothing to do with double-dash, or any double-anything.

@gorhill

This comment has been minimized.

Owner

gorhill commented Dec 24, 2017

tunein.com##[class^="footer__socialSection___"]

or

tunein.com##[class^="footer__socialLink___"]

work fine for me.

@gorhill

This comment has been minimized.

Owner

gorhill commented Dec 24, 2017

tunein.com##div[class*="socialLink"]
tunein.com##div[class^="footer__socialLink"]

These filters don't work because there is no div with class with socialLink substring in it, there are a elements with class footer__socialLink___1Iby-, and there is a div element with a data-testid attribute set to socialLinksContainer.

@ryanbr

This comment has been minimized.

ryanbr commented Dec 24, 2017

hmm annoying, right, doesn't show what type of element it is. Is there feature request to show the type of element being used when using the Picker?

@gorhill

This comment has been minimized.

Owner

gorhill commented Dec 24, 2017

There is #1736, which I interpret as wanting more controls over cosmetic filter crafting.

@gorhill

This comment has been minimized.

Owner

gorhill commented Dec 25, 2017

By the way, how did you come to create the filter with a div with the element picker? The element picker would have found no match on the page, and in such case it won't let you create the filter.

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