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

Initial support for extended CSS selectors (a.k.a. procedural filters) #1574

Merged
merged 2 commits into from
Jan 21, 2021

Conversation

remusao
Copy link
Collaborator

@remusao remusao commented Jan 13, 2021

What Changed

  • Add support for :remove() modifier (:style updated to support procedural filters).
  • Remove stylesheet injection from content script.
  • Replace Closure compiler with Terser.
  • Handle procedural filters:
    • :has and :if
    • :has-text
    • :not
  • Support procedural filters on WebExtension.
  • Improve test coverage.

Release Notes

Add initial support for extended CSS selectors (a.k.a. procedural filters) as well as the :remove() modifier for element hiding rules (note: the already supported :style modified now also works with extended CSS selectors). The following new pseudo-classes are implemented: :has (and its alias :if), :has-text (both string and RegExp literals), and :not (whenever its argument is also an extended selector, otherwise fallback to native implementation).

Caveats:

  • Loading of extended css filters is disabled by default and needs to be toggled using the loadExtendedSelectors option while initializing the blocker instance.
  • These news selectors are currently only supported by WebExtensionBlocker (support for Puppeteer, Electron and Playwright is not planned at this time but help from the community would be greatly appreciated).

Miscellaneous changes:

  • Removal of unused injectCSSRule helper.
  • Replace Closure compiler by Terser.

@remusao remusao added the PR: New Feature 🚀 Increment minor version when merged label Jan 13, 2021
@lgtm-com

This comment has been minimized.

@remusao remusao force-pushed the procedural branch 2 times, most recently from 18a9400 to 6f47e2b Compare January 13, 2021 12:23
expect(querySelectorAll(document.documentElement, ast)).to.have.members(expected);
}

describe('eval', () => {
Copy link
Member

@chrmod chrmod Jan 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a nit: using arrow functions with mocha is not recommended as some functionalities like this.timeout wont work

parse(
'table.tborder > tbody:has(:scope > tr > .alt1 > table > tbody > tr > td > a):has(strong):has(span > font > strong)',
),
).to.eql({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

putting expected value may improve the readability

'type': 'compound',
});

expect(parse('.cls > a > div[class^="foo"]:has(button[class^="bar"])')).to.eql({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors from those expectations may not be very helpful as those would just asset the objects are different while you are interested in the exact difference.
It may be worth to construct a smaller DOM to illustrate individual features in simpler "environment".

// Normal CSS
if (styles && styles.length > 0) {
setTimeout(() => injectCSSRule(styles, window.document), 0);
setTimeout(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setTimeout 0 may be deferred by quite a bit. Consider using postMessage, requestAnimationFrame or Promise.resolve().then if you want to get the callback to fire asap.

On the other hand if a tick is needed it's always better to wait for a sideeffect before calling the "callback".

packages/adblocker-webextension-cosmetics/adblocker.ts Outdated Show resolved Hide resolved
@remusao remusao force-pushed the procedural branch 2 times, most recently from 1e735d1 to ff0fc40 Compare January 21, 2021 10:03
@remusao remusao marked this pull request as ready for review January 21, 2021 10:45
@remusao remusao merged commit e35dc51 into ghostery:master Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: New Feature 🚀 Increment minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants