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

Single invalid style filter disables all cosmetic filtering on the page #2170

Closed
8 tasks done
Yuki2718 opened this issue Jul 13, 2022 · 17 comments
Closed
8 tasks done
Labels
bug Something isn't working fixed issue has been addressed

Comments

@Yuki2718
Copy link

Yuki2718 commented Jul 13, 2022

Prerequisites

I tried to reproduce the issue when...

  • uBO is the only extension
  • uBO with default lists/settings
  • using a new, unmodified browser profile

Description

As the title says. See Steps to reproduce.

A specific URL where the issue occurs

www.backcountry.com

Steps to Reproduce

  1. Add
www.backcountry.com##.chakra-portal
www.backcountry.com##body:style(overflow: auto !important;padding: var(--chakra-space-0 !important;)

to My filters - note the second rule lacks an close parenthesis for var but neither picker or assets viewer marks as invalid. Also enable EasyList Cookie to see the effect.

  1. Visit the page and see newsletter modal and cookie banner are displayed, which should not be the case and the logger shows according filters applied.
  2. If you add the close parenthesis (i.e. www.backcountry.com##body:style(overflow: auto !important;padding: var(--chakra-space-0) !important;)) everything works as expected1.

  1. The style filter acutally doesn't work on the page but it's not relevant here.

Expected behavior

uBO ignores the style filter and other elements are hidden.

Actual behavior

All cosmetic filters are not applied yet logged.

uBlock Origin version

1.43.0/1.43.1b3

Browser name and version

Firefox 102.0.1

Operating System and version

Windows 10

@krystian3w
Copy link

krystian3w commented Jul 14, 2022

Looks like Chrome, Safari and Firefox bug:

https://output.jsbin.com/tiqabalije

So uBo may need implement abstraction with Stylelint like in Stylus. Or move these css to end of tabs.insertCSS() / scripting.insertCSS() resource (like AdGuard addon).


(Bug free are only legacy IE 5 (from 1999-2001) and Safari 5 (from 2012) and maybe similar old FX/Chromium.)

@gwarser
Copy link

gwarser commented Jul 14, 2022

I'm redirected to https://www.bergfreunde.eu/?wt_mc=eu.referral.backcountry.backcountry.-&pid=10004&utm_source=backcountry&utm_medium=referral :/

@krystian3w
Copy link

krystian3w commented Jul 14, 2022

IMO possible rewrite to example.org/com.

example.*##body:style(overflow: scroll !important;padding: var(--scroll !important;)
example.*##body *
  • scroll are enforced
  • Elements inside body are still visible

Or you mean hack on jsbin?

@Yuki2718
Copy link
Author

I'm redirected to https://www.bergfreunde.eu/?wt_mc=eu.referral.backcountry.backcountry.-&pid=10004&utm_source=backcountry&utm_medium=referral :/

Maybe geo-dependent then. I'm accessing from Japan. Or as @krystian3w suggested test on another site.

@krystian3w
Copy link

Definitely EU locked page.

@gorhill
Copy link
Member

gorhill commented Jul 14, 2022

I worked a bit on this yesterday. The crux of the problem is that the browsers silently drop the bad CSS property and everything that follows in the crafted stylesheet. No error is emitted. The solution I found was to inject the CSS property(ies) to test as a stylesheet instead of as a single CSS rule, and then test whether the resulting stylesheet has the number of expected CSS rules (2). I did use replaceSync to accomplish this, but it turns out this is a recently added API, so I can't use this. I will probably have to use HTMLStyleElement.textContent to accomplish this and hope this works as expected -- this is where I am at now.

gorhill added a commit to gorhill/uBlock that referenced this issue Jul 15, 2022
@gwarser
Copy link

gwarser commented Jul 15, 2022

@gorhill style is leaking into the Dashboard???

image

@krystian3w
Copy link

krystian3w commented Jul 15, 2022

Yes sir.

obraz
picker numbers and "<->"
/* something inline */
body * {
  color: red;
}

@gorhill
Copy link
Member

gorhill commented Jul 16, 2022

Oops. I assumed this line would work:

styleElement.disabled = true;

gorhill added a commit to gorhill/uBlock that referenced this issue Jul 17, 2022
@gorhill
Copy link
Member

gorhill commented Jul 17, 2022

Just found out that cosmetic filters with pseudo elements are deemed invalid in Firefox:

##a::before

Is a perfectly valid filter, but it's being rejected in Firefox, while it passes in Chromium.

It seems Firefox's implementation of CSSRule.selectorText is faulty and causing this. There was measurable performance gain in the parser with using CSSRule.selectorText approach, but I will have to fall back to using the generate-a-whole-stylesheet approach.

@Yuki2718
Copy link
Author

But pseudo elements rules have actually been working on Firefox. For example, on https://prtimes.jp/story/detail/YxREvyu63OB if you add

prtimes.jp##.detail__content-sns-area:style(position: static!important)
prtimes.jp##.detail__content-sns-items:not(:first-child):not(:last-child)

the right sidebar lacks the top edge. Adding prtimes.jp##.detail__content-sns-area::after fixes that.

@krystian3w
Copy link

krystian3w commented Jul 18, 2022

But this looks like use pseudo-elements in picker or very generic in file/my filters pane like use DandelionSprout: #464

@gorhill
Copy link
Member

gorhill commented Jul 18, 2022

But pseudo elements rules have actually been working on Firefox

Yes I know, I meant my latest commit here is causing this undue invalidation.

gorhill added a commit to gorhill/uBlock that referenced this issue Jul 18, 2022
@gwarser gwarser added bug Something isn't working fixed issue has been addressed labels Jul 18, 2022
@gwarser gwarser closed this as completed Jul 18, 2022
@krystian3w

This comment was marked as off-topic.

@gorhill
Copy link
Member

gorhill commented Jul 18, 2022

use many lines and move line two into line one and delete no neded characters

Need more specific repro steps. What are those lines? Where do you split in two lines? Which characters to delete?

@krystian3w

This comment was marked as off-topic.

@krystian3w
Copy link

krystian3w commented Aug 8, 2022

@gorhill @Yuki2718 @gwarser Still I can reddish text in uBo interface by:

##something, p
##p, p

##something, :root *

But this may not longer related with fixes added to this issue.

It looks uBo tries mark duplicated selector in filter but fail and colorize sometimes real <element> in interface.

Non HTML5 tags like <something> maybe is treated as * for only "parser".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed issue has been addressed
Projects
None yet
Development

No branches or pull requests

4 participants