Improve/refine syntax highlighter

[gorhill]

Instead of having countless issues opened for every single improvements/refining that could be brought to the syntax highlighter, here is one meta issue to handle them all. I will list all the actionable items in the top comment. Feel free to report un-handled cases or suggest more refinements.

To do:

• Add a filter list which purpose is to validate the static filtering parser
  • Done:
    • https://raw.githubusercontent.com/gorhill/uBlock/master/docs/tests/static-filtering-parser-checklist.txt
    • Subscribe to it and disable it after the list is imported, then use asset viewer to see the result.
• *$xhr,empty should be reported as valid
• The following redirect rules should be reported as invalid:
  • *$beacon,redirect-rule=empty
  • *$ping,redirect-rule=empty
  • *$websocket,redirect-rule=empty
• Detect invalid !# directives
  • Support auto-completion for valid directives
• Detect unmatched !#if-!#endif directives
• Handle line continuation -- I need to read and wrap my mind around CodeMirror's state object
• Detect and fix regex-based filters starting with /.*: they are inefficient and the leading .* accomplishes nothing.
• Ambiguous option separator, see Improve/refine syntax highlighter #1134 (comment).

Nice to have:

• Buttons to navigate to next/prev problematic filter in asset viewer and "My filters
• Warn in gutter about untokenizable static network filters
• Create clickable links for URLs present in comments
  • Implemented as follow: double clicking the URL will select whole URL, at which point you can use your browser's "Open in new tab" feature on the selection.
• Code folding ability:
  • !#if ... / !#endif blocks
  • !#include blocks

[Yuki2718]

Warn in gutter about untokenizable static network filters
This will be huge plus!

[gwarser]

This is invalid now:

$all,~document

easylist/easylist#5621 (comment)

gorhill/uBlock#2385

[gorhill]

I will fix this asap. However I would like to understand the purpose of that /adguard. filter in EasyList. Someone knows?

[krystian3w]

Maybe some ad engine have name "adguard...".

[Yuki2718]

I will fix this asap. However I would like to understand the purpose of that /adguard. filter in EasyList. Someone knows?

Apparently came from the Fanboy's list so @ryanbr would know. Clearly it has nothing to do with AdGuard.
easylist/easylist@648b822

[gwarser]

This look like first occurrence: ryanbr/fanboy-adblock@d51ac92

[gorhill]

If I am getting this right, that /adguard. filter has always been rejected by uBO, since it does not use the all property, ~document can't be used on its own.

I am trying to assess whether I should publish an emergency fix for the all,~document regression.

[gwarser]

~document was added recently easylist/easylist@24784f0

This is puzzling: easylist/easylist#5286

---

[rdk@on assets.json_resources]$ grep '~document' *
DEU-0_DEU EasyList Germany.txt:@@||adonline.anonza.de^$~document,~third-party
easylist_EasyList.txt:/adguard.$~document,domain=~adguard-com.cdn.ampproject.org|~adguard.com|~adguard.mobi|~adguard.oneskyapp.com|~greinr.com|~marketplace.visualstudio.com
easylist_EasyList.txt:@@||lemon-ads.com^$~document,~third-party
[rdk@on assets.json_resources]$ 

[gorhill]

This is puzzling: easylist/easylist#5286

Yeah, /adguard. on its own would never cause strict-blocking to kick in, so it's unclear what the original issue was.

The other cases seem to be about not blocking 1st-party secondary resources without wholly disabling filtering -- so probably they are targeting ABP specifically.

[gwarser]

Yeah, /adguard. on its own would never cause strict-blocking to kick in, so it's unclear what the original issue was.

This was fixed after gorhill/uBlock@41636c5 v1.21 and I think it was fixed very recently in legacy builds. In 1.16.4.22

[gwarser]

Are filters with errors actually dropped? Because:

My filters 1 used out of 1

||testwa-blokada.eu.example^$script,domain=gameplanet.onet.pl/gry-online/

And nothing is logged in the logger.

v1.28.5b1 in Chome.

MajkiIT/polish-ads-filter#16850 (comment)

[gorhill]

That one is not dropped but on the other hand it will never match, since no hostname will ever match gameplanet.onet.pl/gry-online/.

I don't think uBO should drop whole filter because one domain= value is invalid, for example:

||testwa-blokada.eu.example^$script,domain=gameplanet.onet.pl/gry-online/|example.com

Surely we still want to enforce the filter on example.com despite that gameplanet.onet.pl/gry-online/ is not a valid domain.

This is why I chose to treat invalid domain= values as soft error, i.e. the filter will not be dropped, and neither the value (or else that would cause the filter to become broader). It gets complicated with negated invalid hostname though...

[gwarser]

Turns out some people imported uMatrix ruleset recipes as filter list in uBO https://www.reddit.com/r/uBlockOrigin/comments/hy3nsy/rulesets_for_english_websites_breaking_youtube/

Maybe ask for proper header in filter lists and treat host files more strict?

---

Another

randomwebsite.org/* $inline-script

https://www.reddit.com/r/uBlockOrigin/comments/hxorev/ublock_origin_page_loading_issue/fzaguh5/

[gorhill]

randomwebsite.org/* $inline-script

That will be taken care with #1118.

[jspenguin2017]

Here's a suggestion for the filter viewer/editor, although not exactly related to syntax highlighting:

When double-clicking on a domain name, the whole domain should be selected, instead of just the part that was clicked on.

Use cases:

• Quickly select and copy domain
• Quickly select and search domain (double-click, then Ctrl+F)

Current behavior:

Suggested behavior:

---

Also when double-clicking on the suffix of a cosmetic filter, the - character should not break selection.

Current behavior:

Suggested behavior:

[gwarser]

||video-weaver..hls.ttvnw.net/v/playlist/.m3u8$replace=/#EXT-X-DATERANGE:ID="stitched-ad[\s\S](#EXT-X-PROGRAM-DATE-TIME)(?![\s\S]?#EXTINF:\d.\d+\,live)[\s\S]|(#EXT-X-DATERANGE:ID="stitched-ad[\s\S]?(#EXT-X-PROGRAM-DATE-TIME)([\s\S]?(#EXTINF:\d.\d+\,live)))/\$5/

[rusty-snake]

Pipes in regexp of removeparam rules are blue. As there is no regexp highlighting they shouldn't be either.

$removeparam=/^(foo|bar)=/,domain=foo.com|bar.com

[u-RraaLL]

I'm entirely not sure if this is within the purview of this issue, but uBO's highlighter doesn't detect errors within the :is() selector (or :where()).

[gorhill]

uBO uses the browser itself to validate CSS selector. If the browser reports as valid, uBO reports as valid.

---

From console, Chromium says document.querySelector('div:is(1)'); is valid -- no exception is triggered, so it's reported as valid.

[uBlock-user]

https://developer.mozilla.org/en-US/docs/Web/CSS/:where
https://developer.mozilla.org/en-US/docs/Web/CSS/:is

Both are fully supported by the browser vendors as part of the Selectors-4 spec.

[Yuki2718]

Before I forget: DandelionSprout/adfilt#163 (reply in thread)

[krystian3w]

but uBO's highlighter doesn't detect errors

uses the browser itself to validate CSS selector

I suppose RaL try said about bad colorize elements inside ( ) for :is / :where and legacy:

:-moz-any (2011)
:-webkit-any (2011)
:matches() - Note: :matches() was renamed to :is() in CSSWG issue #3258 (2018).

[gwarser]

Anything you put in :is() or :where() will be valid? https://developer.mozilla.org/en-US/docs/Web/CSS/:is#forgiving_selector_parsing

---

https://drafts.csswg.org/selectors-4/#typedef-forgiving-selector-list

To parse as a forgiving selector list given an input input:
...
2. Remove all failure items from selector list, and all items that are invalid selectors, then return a representing the remaining items in selector list. (This might be empty.)

[gwarser]

Can errors be marked by text-decoration: red wavy underline;? It will make them more distinguishable from tokenization issues and make it more in line with how text editors mark incorrect spelling.

[gorhill]

I tried to use wavy originally, but this not work well in CodeMirror.

Eventually I plan to add a gutter markers to better convey extra information.

[gwarser]

Not pretty on Chrome, but it's not broken so badly on my side:

Nightly 95 uBO dev vs Chrome 95 uBO stable

Different font?

[krystian3w]

Or this is ugly https://developer.mozilla.org/en-US/docs/Web/CSS/text-decoration-skip-ink

[krystian3w]

https://developer.mozilla.org/en-US/docs/Web/CSS/text-underline-position

https://developer.mozilla.org/en-US/docs/Web/CSS/text-underline-offset

[MasterKia]

*$1p,strict3p,script,header=via:1.1 google is highlighted correctly in "My filters"

But the header=via:1.1 google part is red in assets viewer.

filterOnHeaders is set to true.

STR:

• Import https://raw.githubusercontent.com/MasterKia/PersianBlocker/main/PersonalBlocker.txt
• Click on the "View content" button.

[MasterKia]

##+js and :matches-path highlighting problem. Not sure if it's worth fixing though.

example.com##+js(noeval):matches-path(/some-path)

example.com##+js(noeval, x):matches-path(/some-path)

[gorhill]

It's not a highlighting issue, since the scriptlet is interpreted as:

example.com##+js(noeval, "x):matches-path(/some-path")

Which is valid.

matches-path is a procedural operator for cosmetic filtering purpose, it's completely unrelated to scriptlet injection.

[uBlock-user]

There are still many tasks unfinished in #1134 (comment), are they abandoned since you closed this issue ?

[garry-ut99]

Improve extraction of tokens from regex-based filters - worth to mention also about this commit, as a part of it is related to untokenizable regex syntax highlighting (and it doesn't contain any reference to this thread, nor any key words in the title, like: syntax, highlight, linter).