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

Add support for ":nth-ancestor" and ":upward" #224

Closed
DandelionSprout opened this issue Feb 18, 2020 · 34 comments · Fixed by #252
Closed

Add support for ":nth-ancestor" and ":upward" #224

DandelionSprout opened this issue Feb 18, 2020 · 34 comments · Fixed by #252
Labels
enhancement New feature or request fixed

Comments

@DandelionSprout
Copy link
Contributor

DandelionSprout commented Feb 18, 2020

While I haven't tested it in Pale Moon (nor SeaMonkey) myself, I think uBO added the new ":nth-ancestor" feature to the regular uBO versions sometime in the past year, and I'm already liking it enough that I'm predicting it'll become a major feature in uBO-specific lists within the next year or two.

So I presume that adding support for it would be advantageous for you guys.

@JustOff
Copy link
Collaborator

JustOff commented Feb 19, 2020

Wiki: subject:nth-ancestor(n)

Current usage:

cd ../uAssets/filters;  grep nth-ancestor * | wc -l
28

@JustOff JustOff added the help wanted Extra attention is needed label Feb 19, 2020
@krystian3w
Copy link

Some diffrence beetwen kth-ancestor?

@DandelionSprout
Copy link
Contributor Author

One key difference is that :kth-ancestor with a K is not supported by any known version of uBO or Nano, I would presume.

As for the mathematical/coding difference, I'm not really sure. If anyone with a university math education can figure out this article or anything similar, that'd be more helpful than anything I could've said.

@THEtomaso
Copy link

uBlock filters already contains a bunch of rules, which uses nth-ancestor.
For the time being, perhaps it would be possible to add more code to the converter, thereby turning those rules into xpath syntaxes?

@JustOff
Copy link
Collaborator

JustOff commented Feb 23, 2020

perhaps it would be possible to add more code to the converter

Someone can try, but it will not be me.

@THEtomaso
Copy link

THEtomaso commented Feb 23, 2020

Currently, the highest n-value for nth-ancestor in uBlock filters is only 5.
I could be wrong, but wouldn't it simplify things a lot, if you were to add 5 separate convertion rules, like this?:

:nth-ancestor(1) = :xpath(..)
:nth-ancestor(2) = :xpath(../..)
:nth-ancestor(3) = :xpath(../../..)
:nth-ancestor(4) = :xpath(../../../..)
:nth-ancestor(5) = :xpath(../../../../..)

..or perhaps add a few more, to be on the safe side?:

:nth-ancestor(6) = :xpath(../../../../../..)
:nth-ancestor(7) = :xpath(../../../../../../..)
:nth-ancestor(8) = :xpath(../../../../../../../..)
:nth-ancestor(9) = :xpath(../../../../../../../../..)

@JustOff
Copy link
Collaborator

JustOff commented Feb 23, 2020

Well, you can play around with it: uBlock0_1.16.4.19b1.firefox-legacy.xpi.zip (rename to xpi).

@THEtomaso
Copy link

Don't know much about this stuff, I'm afraid.
I've edited XPI files before, but only to apply hacks of the most brutal kind! :)

@DandelionSprout
Copy link
Contributor Author

I remember experimenting with the uBO XPI once before to try to copy over entire files from the regular branch, which didn't go so well (It didn't take much changes for the entire extension to cease working entirely), but I presume JustOff intended for the changes to be made to the one single same file that the ##+js conversions were added to?

I could've plausibly done it myself, but I've had a pretty tiring day today.

@JustOff
Copy link
Collaborator

JustOff commented Feb 23, 2020

Don't know much about this stuff, I'm afraid.

I gave you already patched xpi with your rules added.

@THEtomaso
Copy link

Ah, so that's what it was.
I'll give it a try! :)

@THEtomaso
Copy link

THEtomaso commented Feb 23, 2020

Seems to work just fine! 👍
How many convertion rules did you add?
9?

@JustOff
Copy link
Collaborator

JustOff commented Feb 23, 2020

Yes, 9, and we can use this as a workaround, right?

@THEtomaso
Copy link

THEtomaso commented Feb 23, 2020

Yes, all rules looks to have been converted correctly, and I saw them being applied in the logger too.
Although, I couldn't exactly pinpoint their effects (most likely because I'm using a very large amount of filters).

@JustOff
Copy link
Collaborator

JustOff commented Feb 23, 2020

Well, @DandelionSprout, @krystian3w, what do you think about this workaround? Is it correct and worth including to the next uBO-legacy release?

@DandelionSprout
Copy link
Contributor Author

After 15min of setting up a test setup, it seems that tek.no##div[id$=_body_ad]:nth-ancestor(1) in my Nordic list was converted to tek.no##div[id$=_body_ad]:xpath(..), which successfully removes a few small ad placeholders at https://www.tek.no/nyheter/nyhet/i/Wb1ela/derfor-fikk-mange-1-tall-pa-mobilen-torsdag.

So my personal standpoint is that the workaround actually works, if that accounts for anything. So I presume it's includable.

JustOff added a commit that referenced this issue Feb 24, 2020
@JustOff
Copy link
Collaborator

JustOff commented Feb 24, 2020

Thanks everyone for testing, I pushed this workaround to the master for the next release.

@gwarser

This comment has been minimized.

@DandelionSprout

This comment has been minimized.

@THEtomaso

This comment has been minimized.

@JustOff

This comment has been minimized.

@uBlock-user
Copy link
Contributor

uBlock-user commented Mar 8, 2020

FYI :nth-ancestor will be deprecated for :upward soon -- gorhill/uBlock@72bb700

@JustOff
Copy link
Collaborator

JustOff commented Mar 8, 2020

Thank you for letting us know about this. This will probably lead to a new workaround, unless of course someone smarter joins the uBO-legacy improvement to add support for new operators properly.

@smnthermes

This comment has been minimized.

@JustOff JustOff changed the title Add support for ":nth-ancestor" Add support for ":upward" Jun 20, 2020
@JustOff JustOff changed the title Add support for ":upward" Add support for ":nth-ancestor" and ":upward" Jun 28, 2020
@JustOff
Copy link
Collaborator

JustOff commented Jul 5, 2020

Since #240, among other things, introduces native support of :nth-ancestor(n), here is another test version, where I removed mapping of :nth-ancestor(n) to :xpath(..) from the syntax converter and also changed mapping of :upward(n) from :xpath(..) to :nth-ancestor(n):

        ':upward(1)': ':nth-ancestor(1)',
        ':upward(2)': ':nth-ancestor(2)',
        ':upward(3)': ':nth-ancestor(3)',
        ':upward(4)': ':nth-ancestor(4)',
        ':upward(5)': ':nth-ancestor(5)',
        ':upward(6)': ':nth-ancestor(6)',
        ':upward(7)': ':nth-ancestor(7)',
        ':upward(8)': ':nth-ancestor(8)',
        ':upward(9)': ':nth-ancestor(9)'

uBlock0_1.16.4.23b2.firefox-legacy.xpi.zip

@JustOff
Copy link
Collaborator

JustOff commented Jul 17, 2020

@THEtomaso
Could you please take a look at uBlock0_1.16.4.24b2.firefox-legacy.xpi.zip based on PR #252 which introduces full native support of :upward and also implements :remove.

@THEtomaso
Copy link

THEtomaso commented Jul 17, 2020

One thing that I noticed right away, is that the :upward rules are completely messed up in uBO's logger!

Example:

Dandelion Sprout's Nordic filters contains this rule:
730.no##.gofollow:upward(3)

But when opening 730.no, this is shown in the logger:
####.gofollow:upward(3)
(4 number signs)

Also, when clicking on a :upward entry in the logger, it doesn't show which filter the rule originates from.
Instead, it simply shows a blank window!

--

EDIT:
The same issues occur with :remove rules.

@JustOff
Copy link
Collaborator

JustOff commented Jul 17, 2020

Please try uBlock0_1.16.4.24b3.firefox-legacy.xpi.zip

@THEtomaso
Copy link

Well, that seems to have fixed the logger issue for :upward rules.

But now, :remove rules doesn't show up in the logger at all!
Example:
navi.z0i.net##.adsbygoogle:remove() @ http://navi.z0i.net/

@JustOff
Copy link
Collaborator

JustOff commented Jul 17, 2020

uBlock0_1.16.4.24b4.firefox-legacy.xpi.zip ?

@THEtomaso
Copy link

THEtomaso commented Jul 17, 2020

Yes, that did the trick! 👍
Now, if I only could figure out what those extremely rare :remove rules does, perhaps we would be able to confirm that they actually works too. :)

@JustOff
Copy link
Collaborator

JustOff commented Jul 17, 2020

Yes, that did the trick!

Thanks for the confirmation!

if I only could figure out what those extremely rare :remove rules does

I can confirm it works, you can try this one:

youtube.com##[page-subtype="channels"] > #contents #player-container:remove()

and open any YT channel, f.e. https://www.youtube.com/user/NationalGeographic

@THEtomaso
Copy link

Indeed, it works!
As usual; many thanks to the both of you, hawkeye116477 and JustOff! 👍

@JustOff JustOff added enhancement New feature or request fixed and removed help wanted Extra attention is needed labels Jul 24, 2020
@THEtomaso
Copy link

@hawkeye116477:
Is this fix directly appliable for the Legacy branch?:
gorhill/uBlock@7dd48a6

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants