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

Properly report :spath operator of procedural cosmetic filters in logger #453

Closed
6 of 8 tasks
krystian3w opened this issue Mar 7, 2019 · 15 comments
Closed
6 of 8 tasks
Labels
bug Something isn't working fixed issue has been addressed

Comments

@krystian3w
Copy link

krystian3w commented Mar 7, 2019

Prerequisites

  • I verified that this is not a filter issue
  • This is not a support issue or a question
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue
    • Your issue may already be reported.
  • I tried to reproduce the issue when...
    • uBlock Origin is the only extension
    • uBlock Origin with default lists/settings
    • using a new, unmodified browser profile
  • I am running the latest version of uBlock Origin
  • I checked the documentation to understand that the issue I report is not a normal behavior

Description

Block someting with xpath and use comma after xpath:

github.com###issue-418448972:xpath(..), blablacar

Try find logger and you found:

Logger output
+4 ###issue-418448972:xpath(..):spath(, blablacar) github.com dom https://github.com/uBlockOrigin/uBlock-issues/issues/453

A specific URL where the issue occurs

#453

Steps to Reproduce

  1. open this issue Properly report :spath operator of procedural cosmetic filters in logger #453
  2. create filter by element picker ##.js-comment-body:xpath(..), blablacar
  3. refresh page and possible in ubo logger you found ##.js-comment-body:xpath(..):spath(, blablacar)

Expected behavior:

mark filter as invalid?

or add works exception + marks spath as valid operator:

#@##issue-418448972:xpath(..):spath(, li)

Actual behavior:

other selector moved to :spath(...) to not discard filter? and expception without remove spath does not work.

Your environment

  • uBlock Origin version: Windows 7
  • Browser Name and version: Firefox 65
  • Operating System and version: uBO 1.18.5rc3
@gwarser gwarser added the bug Something isn't working label Mar 7, 2019
@gwarser
Copy link

gwarser commented Mar 7, 2019

:spath is to support chaining back from procedural to native CSS selectors

In github.com###issue-418448972:xpath(..) div.avatar-parent-child, div.avatar-parent-child should be in "spath".

  • I was sure it's only internal and should not be visible in logger (?). Should be added to wiki "Procedural cosmetic filters"?
  • comma is confusing filter parser

@krystian3w
Copy link
Author

Also strange is exception with :spath does not working?

@mapx-
Copy link
Contributor

mapx- commented Mar 7, 2019

@gwarser it's about https://github.com/orgs/uBlockOrigin/teams/ublock-issues-volunteers/discussions/59 , right ?

@mapx-
Copy link
Contributor

mapx- commented Mar 7, 2019

it wasn't implemented support of comma-separated procedurals

@gwarser
Copy link

gwarser commented Mar 7, 2019

it's about...

Yep, this is it.

gorhill on Dec 30, 2018 •

I didn't implement support of comma-separated procedurals, an issue will have to be opened about this, and given that the workaround is to simply declare each on its own line, it's unlikely to be something I will work on this for now. That is unless such filters become a common occurrence in other lists (or if I figure a trivial fix to this).

But I still want to know if :spath should be displayed in the logger. If yes, it should be added to the wiki to tell people what it is.

@gwarser
Copy link

gwarser commented Mar 7, 2019

Also strange is exception with :spath does not working?

? You mean, something like this: #@#.js-comment-body:xpath(..):spath(.real-class) ?

@gwarser gwarser reopened this Mar 7, 2019
@krystian3w
Copy link
Author

krystian3w commented Mar 7, 2019

Yes, I need remove ":spath( " (with space), and ")" form filter generated by uBO logger/ uBO dom inspector, otherwise it does not work as exception.


github.com##.js-comment-body:xpath(../../../../../../../..) div.avatar-parent-child

no recover avatars:
github.com#@#.js-comment-body:xpath(../../../../../../../..):spath( div.avatar-parent-child)

@gorhill
Copy link
Member

gorhill commented Mar 7, 2019

Don't use :spath, it's for internal use only.

@gorhill gorhill closed this as completed Mar 7, 2019
@krystian3w
Copy link
Author

krystian3w commented Mar 7, 2019

I am no use, Logger "recommed" exception with spath.

@uBlock-user uBlock-user added invalid not a uBlock issue and removed bug Something isn't working labels Mar 7, 2019
@gorhill gorhill reopened this Mar 7, 2019
@gorhill gorhill removed the invalid not a uBlock issue label Mar 7, 2019
@uBlock-user uBlock-user added the bug Something isn't working label Mar 8, 2019
@gorhill
Copy link
Member

gorhill commented Mar 8, 2019

Sorry for having closed this too fast yesterday, I was tired and didn't understand properly the reported issue.

So the first problem is that uBO reports :spath in the logger. I use the following filter:

news.ycombinator.com##tr:has(> * > .storylink) + tr

On the front page of https://news.ycombinator.com/.

gorhill added a commit to gorhill/uBlock that referenced this issue Mar 8, 2019
@krystian3w
Copy link
Author

No problem, worse should by banned in repo.

@krystian3w krystian3w changed the title :spath(...) in uBO logger? Properly report :spath operator of procedural cosmetic filters in logger Mar 8, 2019
@gwarser
Copy link

gwarser commented Mar 8, 2019

:spath in logger, DOM inspector and exceptions is fixed.

Comma separated filters are displayed in logger, but are ignored (not applied to page)


Clarification: first filter from comma-separated list is applied, second filter is ignored.

@gorhill
Copy link
Member

gorhill commented Mar 8, 2019

I tested the comma case, and I was surprised that it was handled without exception -- though I have no clue whether the result make sense (quite probably not).

Given that fixing this is less trivial than fixing the logger reporting (which fix I felt was safe to include in current release candidate) , I prefer to fix this separately as another issue, not to be fixed for the next release.

@uBlock-user uBlock-user added the fixed issue has been addressed label Mar 8, 2019
@gwarser
Copy link

gwarser commented Mar 8, 2019

What I tried was: ###issue-418448972:xpath(..) div.avatar-parent-child, .sidebar-labels-style. This should block OP avatar in first message and labels on the sidebar ("bug", "fixed").

Avatar is hidden, labels are not. Filter is not logged as invalid in the logger. Filter ending with comma is logged as invalid (###issue-418448972:xpath(..) div.avatar-parent-child,)

@gorhill
Copy link
Member

gorhill commented Mar 9, 2019

Avatar is hidden, labels are not.

I spent some time looking into this. Here is what is happening. When an "spath" is appended to a procedural operator chain, here is what uBO internally does:

Create a plain CSS selector matching the nodes selected as a result of the procedural operator. The CSS selector is of the form p:nth-child(n), where p is the parent node, and n is the rank of the selected node in the parent.

Then the plain CSS selector is appended to this p:nth-child(n), so in case of comma usage, that would be one of the 3 following CSS selectors:

  • p:nth-child(n), .toto
  • p:nth-child(n) .toto, .titi
  • p:nth-child(n) .toto,

The 3rd-form will be detected as invalid. However the 1st and 2nd form will be detected as valid. uBO could detect the 1st form easily and discard it as invalid. The 2nd form however can't be trivially detected -- this would require uBO to create some sort of CSS parser just to handle this.

Because of this, I've decided I won't add code to care about commas for such cases. In the end, I consider that using commas in the middle or end of a procedural operator chain is contrived (in the case here this was used only to show an issue with logger output), and the result should be assumed to be undefined.

hawkeye116477 pushed a commit to hawkeye116477/uBlock-for-firefox-legacy that referenced this issue Jul 4, 2020
@krystian3w krystian3w changed the title Properly report :spath operator of procedural cosmetic filters in logger Properly report :spath operator of procedural cosmetic filters in logger Feb 2, 2023
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

5 participants