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

Adding the @media operator #2185

Closed
8 tasks done
dimisa-RUAdList opened this issue Jul 22, 2022 · 33 comments
Closed
8 tasks done

Adding the @media operator #2185

dimisa-RUAdList opened this issue Jul 22, 2022 · 33 comments
Labels
enhancement New feature or request fixed issue has been addressed

Comments

@dimisa-RUAdList
Copy link

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

Sometimes the layout of a page changes depending on its width.

For example: https://mail.ru

Screenshot

mail

In this case, the same cosmetic rules act differently. This leads to the need to have an adaptive way of applying the rules.

I propose to add an operator for @media: https://developer.mozilla.org/en/docs/Web/CSS/@media

A specific URL where the issue occurs

https://mail.ru

Steps to Reproduce

unknow

Expected behavior

unknow

Actual behavior

unknow

uBlock Origin version

uBlock Origin development build 1.43.1b7

Browser name and version

Google Chrome 103.0.5060.134

Operating System and version

Windows 7

@gorhill
Copy link
Member

gorhill commented Jul 22, 2022

What would be the specific filter you need here?

@dimisa-RUAdList
Copy link
Author

In this case, you need to get rid of the logo branding (VK). For this I planned to use:

mail.ru##.grid__lcol > .mailbox-container > .logo > .logo__img--vk:style(background-size: auto 40px !important; background-position: 135% 50% !important)
mail.ru##.grid__lcol > .mailbox-container > .logo > .logo__img--vk > .logo__text:style(display: block !important; width: 78px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)

But when the width changes, these rules don't move the target elements as intended.

Yes, I know, can use:
mail.ru##+js(remove-class, logo__img--vk, .logo__img, complete)

But this results in flickering of the logo, which can be just as annoying for users as the branding. Partially flickering can be removed with:
mail.ru##.grid__lcol .logo__img--vk:style(opacity: 0 !important)

But even so, there will be a delay in the appearance of the logo. So I had the idea of adding the @media operator.

Then the rule will look like:
mail.ru##@media (min-width: *****px) .grid__lcol ***

@gorhill
Copy link
Member

gorhill commented Jul 22, 2022

Then the rule will look like:
mail.ru##@media (min-width: *****px) .grid__lcol ***

How about:

mail.ru##:matches-media(min-width: *****px) .grid__lcol ***

JS-side, uBO would just leverage Window.matchMedia to implement the operator, in which case a match would result in the operator being a passthru, and a non-match would result in an empty set.

@dimisa-RUAdList
Copy link
Author

The syntax for me is not of fundamental importance, the main thing is the very possibility of using a new operator.

gorhill added a commit to gorhill/uBlock that referenced this issue Jul 23, 2022
Related issue:
- uBlockOrigin/uBlock-issues#2185

The argument must be a valid media query as documented on MDN, i.e.
what appears between the `@media` at-rule and the first opening
curly bracket (including the parentheses when required):
- https://developer.mozilla.org/en-US/docs/Web/CSS/Media_Queries/Using_media_queries

Best practice:

Use `:matches-media()` after plain CSS selectors, if any.

Good:
    example.com###target-1 > .target-2:matches-media((min-width: 800px))

Bad (though this will still work):
    example.com##:matches-media((min-width: 800px)) #target-1 > .target-2

The reason for this is to keep the door open for a future optimisation
where uBO could convert `:matches-media()`-based filters into CSS media
rules injected declaratively in a user stylesheet.
@uBlock-user
Copy link
Contributor

Duplicate of #2100 ?

@gorhill
Copy link
Member

gorhill commented Jul 23, 2022

I don't know whether this would also address #2100, I haven't investigated whether the browser properly runs content scripts when rendering the page for printing. The request here is to address filtering issues, while #2100 was not a filtering issue but rather a personal request which I generally decline.

@uBlock-user
Copy link
Contributor

seems we can't set display property via :matches-media() -

example.com##body:matches-media((display: block)) appears invalid

This is for a case where body is hidden when trying to print - https://gakufu.tunegate.me/music/CT00379

because of a style rule present in inline style -

@media print {
    body { display: none !important; }
} 

@gorhill
Copy link
Member

gorhill commented Jul 23, 2022

I don't see (display: block) as being a valid media query, there is no documentation for display.

@gorhill
Copy link
Member

gorhill commented Jul 23, 2022

This works:

gakufu.tunegate.me##body:style(display: block !important)

@uBlock-user
Copy link
Contributor

Yeah it's used with print here but unfortunately they haven't document any example with that.

gakufu.tunegate.me##body:style(display: block !important)

That works, very nice.

@gorhill
Copy link
Member

gorhill commented Jul 23, 2022

because of a style rule present in inline style -

If you want to apply specific style for specific media query, you need to use the form:

example.com##body:matches-media(print):style(display: block)

In any case, this doesn't work for the specific case you provide because I don't think procedural operators are re-run when printing, I believe the browser just starts to apply declared @media print rules internally. For uBO to work in this case would require to convert :matches-media operator-based filters into declarative CSS rules, something which I mention in the commit, but this would require more work.

@dimisa-RUAdList
Copy link
Author

Unfortunately, the new operator :matches-media() does not work as well as I expected. The flickering of the logo is observed, characteristic of the remove-class. It is this flickering that I wanted to avoid with the new operator.

Video uBO media

ubo media

I used:

mail.ru##.grid__lcol:matches-media((min-width: 1317px)) .logo__img--vk:style(background-size: auto 40px !important; background-position: 135% 50% !important)
mail.ru##.grid__lcol:matches-media((min-width: 1317px)) .logo__img--vk > .logo__text:style(display: block !important; width: 78px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)

If we add similar rules to AdGuard:

mail.ru#$#@media (min-width: 1317px) {.grid__lcol > .mailbox-container > .logo > .logo__img--vk {background-size: auto 40px !important; background-position: 135% 50% !important;}}
mail.ru#$#@media (min-width: 1317px) {.grid__lcol > .mailbox-container > .logo > .logo__img--vk > .logo__text {display: block !important; width: 78px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important;}}

You can see that the rules work instantly, and there is no flicker.

Video AdGuard media

adg media

Is it possible to achieve the same smooth operation with uBO?

@gorhill
Copy link
Member

gorhill commented Jul 23, 2022

Is it possible to achieve the same smooth operation with uBO?

I've been working in the mean time to generate declarative CSS where possible when using :matches-media(), and this seems to work. The problem though is that the logger doesn't handle well such declarative :matches-media() occurrences and I don't think I want to get into this now, so what I can do for now is support declarative :matches-media() where possible but these instances won't be reported in the logger.


By the way, for this to work this has to follow what I says in the release notes, so in your case:

mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1317px)):style(...)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1317px)):style(...)

uBO will convert to use declarative form only when :matches-media() is immediately followed by :style().

@dimisa-RUAdList
Copy link
Author

With this kind of rule, the logo still flickers. And as I said, it is the flickering that I am trying to get rid of. But I still want to thank you for the speed of implementation.

@gorhill
Copy link
Member

gorhill commented Jul 23, 2022

the logo still flickers

Yes, the improvements are still not committed.

@gwarser gwarser added the enhancement New feature or request label Jul 24, 2022
@dimisa-RUAdList
Copy link
Author

I installed version 1.43.1b9 and now the rules work instantly. But when resizing the window, when I use several rule options, they start to interfere with each other.

! min-width: 1521px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1521px)):style(background-size: auto 40px !important; background-position: 90px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1521px)):style(display: block !important; width: 78px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1317px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1317px)):style(background-size: auto 40px !important; background-position: 90px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1317px)):style(display: block !important; width: 78px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1253px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1253px)):style(background-size: auto 40px !important; background-position: 75px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1253px)):style(display: block !important; width: 60px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1189px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1189px)):style(background-size: auto 40px !important; background-position: 75px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1189px)):style(display: block !important; width: 60px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1069px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1069px)):style(background-size: auto 40px !important; background-position: 60px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1069px)):style(display: block !important; width: 60px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)

@gorhill
Copy link
Member

gorhill commented Jul 24, 2022

they start to interfere with each other.

There is no guarantee of order about where the rules will appear in the generated stylesheet. You will need to use (min-width: ...px) and (max-width: ...px) if you want to be sure one single CSS rule apply on a given width range.

@dimisa-RUAdList
Copy link
Author

dimisa-RUAdList commented Jul 24, 2022

I got it like this:

! min-width: 1317px and more >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((min-width: 1317px)):style(background-size: auto 40px !important; background-position: 90px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((min-width: 1317px)):style(display: block !important; width: 78px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1253px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((max-width: 1316px)):style(background-size: auto 40px !important; background-position: 75px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((max-width: 1316px)):style(display: block !important; width: 60px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1189px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((max-width: 1252px)):style(background-size: auto 40px !important; background-position: 75px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((max-width: 1252px)):style(display: block !important; width: 60px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)
! min-width: 1069px >>>
mail.ru##.grid__lcol .logo__img--vk:matches-media((max-width: 1188px)):style(background-size: auto 40px !important; background-position: 60px 50% !important)
mail.ru##.grid__lcol .logo__img--vk > .logo__text:matches-media((max-width: 1188px)):style(display: block !important; width: 60px !important; height: 40px !important; opacity: 1 !important; position: absolute !important; right: 0 !important; top: 15px !important; background-color: #005ff9 !important)

Is everything right?

@krystian3w
Copy link

Maybe possible move stable css declarations outside matches-media (css minify operation).

Then will override unstable width and background-position (also I found background-position-x in CSS spec).

@dimisa-RUAdList
Copy link
Author

dimisa-RUAdList commented Jul 25, 2022

@gorhill
Found a problem. In version 1.43.1b9 + RU AdList + BitBlock, the rule stopped working:
rbc.ru,sportrbc.ru##.l-sticky:has(.forecast:empty) ~ .js-news-feed > .news-feed:style(top: 166px !important)

Example: https://www.rbc.ru/

The purpose of this rule is to remove the indentation in the left column, in cases where the forecast is blocked (using BitBlock).

At the same time, when using the Element picker, the rule works, but only until the page is refreshed.

@uBlock-user
Copy link
Contributor

@dimisa-RUAdList try force updating the list where the rule is present.

@dimisa-RUAdList
Copy link
Author

@uBlock-user
The problem is not the lack of an up-to-date version of the filter. The rule is displayed in the logger. And the rule does not work, even if you add it to My filters.

@uBlock-user
Copy link
Contributor

The problem is not the lack of an up-to-date version of the filter.

My issue was, after the update, one :remove() filter couldn't be disabled any way whatsoever, so force updated the list and the regression went away.

@dimisa-RUAdList
Copy link
Author

dimisa-RUAdList commented Jul 25, 2022

@uBlock-user
This is probably some other problem not related to rbc.ru.

@krystian3w
Copy link

Maybe now procedural css is rechecked too rare.

PS. These still fine: #89 & #1247 so regression isn't huge.

@gorhill
Copy link
Member

gorhill commented Jul 25, 2022

the rule stopped working

Found the issue, caused by some changes I made. I will publish a fix shortly.

gorhill added a commit to gorhill/uBlock that referenced this issue Jul 25, 2022
@dimisa-RUAdList
Copy link
Author

@gorhill
Now everything works as it should. Both on mail.ru and rbc.ru. Great job!

@gorhill
Copy link
Member

gorhill commented Oct 19, 2022

Not yet supported by csstree: csstree/csstree#143

@MasterKia
Copy link
Member

This is for a case where body is hidden when trying to print - https://gakufu.tunegate.me/music/CT00379
because of a style rule present in inline style -

@uBlock-user Using replace-node-text, you can use one of these:

example.com##+js(rpnt, style, /@media\s+print\s*\{([^{]+\{[^}]+\}[^}]+)+?\}\s*\}/)
or
example.com##+js(rpnt, style, /@media print\{(.*)\}/)

@uBlock-user uBlock-user added the fixed issue has been addressed label Jun 16, 2023
@uBlock-user
Copy link
Contributor

@MasterKia Currently, tunegate.me##style:has-text(@media print):remove() is taking care of the issue. So if you want to replace the filter with one of yours, go ahead.

@MasterKia
Copy link
Member

MasterKia commented Jun 16, 2023

replace-node-text is suitable for sites that use a single style tag for all of their CSS where style:remove() will break the site layout.

@krystian3w
Copy link

krystian3w commented Jun 16, 2023

For these should be converted:

https://www.postcourier.com.pg

postcourier.com.pg family pages
pitesti24.ro,samsungtechwin.com,cours-de-droit.net,iptv4best.com,blogvisaodemercado.pt,kapitalis.com,tiempo.hn,winmeen.com,ibps.in,visse.com.br,javsubtitle.co,licensekeys.org,mediahiburan.my,tipssehatcantik.com,jbjbgame.com,viatasisanatate.com,ziarulargesul.ro,globaldefensecorp.com,gossipnextdoor.com,coffeeapps.ir,media.framu.world,immobiliaremia.com,colegiosconcertados.info,bigdatauni.com,rukim.id,visefierbinti.ro,theaircurrent.com,nocturnetls.net,clockks.com,ananda-yoga.ro,poolpiscina.com,infodifesa.it,getective.com,formatatmak.com,drkrok.com,alphagirlreviews.com,kitchennovel.com,voxvalachorum.ro,cracksone.com,day-hoc.org,onlineonderdelenshop.nl,primicia.com.ve,tech-recipes.com,afrikmag.com,maduras.vip,aprendeinglessila.com,kicknews.today,koalasplayground.com,hellokpop.com,hayatbilgileri.com,moneyexcel.com,placementstore.com,neuroteam-metz.de,codedosa.com,liveyourmaths.com##style:has-text(@media print):remove()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed issue has been addressed
Projects
None yet
Development

No branches or pull requests

6 participants