From 1e77dac2a66bb09ba9b001e9712f3c332099f2a3 Mon Sep 17 00:00:00 2001 From: Slava Leleka Date: Mon, 3 Oct 2022 19:40:36 +0300 Subject: [PATCH] AG-16255 fix comments, add limitations to readme Squashed commit of the following: commit 6606a52c0c0be6f7a6b18f7dd896431c7b7de1ca Author: Slava Leleka Date: Thu Sep 29 14:12:49 2022 +0300 add remove limitations to readme commit 4374b850102e85d2ab6479d0437a3f88461d7af1 Author: Slava Leleka Date: Thu Sep 29 13:19:47 2022 +0300 fix grammar commit b50e2d55b5993a4db9d0cd0d67630a336d2027a1 Author: Slava Leleka Date: Thu Sep 29 13:18:07 2022 +0300 remove todo comment about limitations commit 9cb1e6b0f38194f8a9ef800621187ac5ee8638cb Author: Slava Leleka Date: Wed Sep 28 20:08:31 2022 +0300 fix few comments commit 1990c66480444a99fa6eb15ddc9faca141c07a97 Author: Slava Leleka Date: Wed Sep 28 20:07:55 2022 +0300 add Limitations to readme commit 487b933a2e5330642e0b7f7d41f7f7a1c4428b46 Author: Slava Leleka Date: Wed Sep 28 19:08:19 2022 +0300 fix comments for test/helpers/selector-parser.ts commit 3d5d47f8896019fea64c8e0b1f7264da49d74105 Author: Slava Leleka Date: Wed Sep 28 18:53:08 2022 +0300 style multiple comments better commit 6676638adc8176ad5e712bcf31b44f2bbf218a5a Author: Slava Leleka Date: Wed Sep 28 18:49:55 2022 +0300 add better comments for stylesheet parser commit 1c3c81ba6c4eeb7f3c998f15809e64690c3b5bf9 Author: Slava Leleka Date: Wed Sep 28 18:18:08 2022 +0300 fix comments for class ExtendedCss commit 90c6e9610e2ccdbc5256730bf347337a112bbd09 Author: Slava Leleka Date: Wed Sep 28 16:52:44 2022 +0300 fix comment for ExtCssConfiguration.debug commit cd06038eb17e9d9e5e6c3fe2e8810615d00eda8f Author: Slava Leleka Date: Wed Sep 28 16:48:24 2022 +0300 fix comment for ExtCssConfiguration.styleSheet commit 57011dedead0925a37448075a894a4a67e3e3489 Author: Slava Leleka Date: Wed Sep 28 16:40:05 2022 +0300 add comment for ExtendedCss Context commit 883b06048a9ce77292286ff7e1c64326771fc851 Author: Slava Leleka Date: Wed Sep 28 16:37:55 2022 +0300 fix comments for ExtCssConfiguration commit 2aced3365887c8708e121c79305619c30a461517 Author: Slava Leleka Date: Wed Sep 28 16:18:29 2022 +0300 fix comments for RelativePredicateArgsInterface --- README.md | 39 +++++++++++++------ src/extended-css/extended-css.ts | 53 ++++++++++++++++++++++++-- src/extended-css/helpers/types.ts | 3 ++ src/selector/parser.ts | 7 +++- src/selector/utils/absolute-matcher.ts | 16 ++++++-- src/selector/utils/query-helpers.ts | 26 ++++++++++--- src/stylesheet/parser.ts | 50 +++++++++++++++++++++++- test/helpers/selector-parser.ts | 42 +++++++++++++++----- test/helpers/selector-query-jsdom.ts | 27 ++++++++++--- 9 files changed, 220 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 11757049..bd06933a 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ AdGuard's ExtendedCss library for applying CSS styles with extended selection properties. * [Extended capabilities](#extended-capabilities) + * [Limitations](#extended-css-limitations) * [Pseudo-class :has()](#extended-css-has) * [Pseudo-class :if-not()](#extended-css-if-not) * [Pseudo-class :contains()](#extended-css-contains) @@ -32,23 +33,24 @@ AdGuard's ExtendedCss library for applying CSS styles with extended selection pr ## Extended capabilities -> Extended pseudo-class should specify the element selection at the end of it's selector representation after the standard part. So `div[class="ad"]:has(img)` is valid but `div:has(img)[class="ad"]` is not. - > Some pseudo-classes does not require selector before it. Still adding a [universal selector](https://www.w3.org/TR/selectors-4/#the-universal-selector) `*` makes an extended selector easier to read, even though it has no effect on the matching behavior. So selector `#block :has(> .inner)` works exactly like `#block *:has(> .inner)` but second one is more obvious. > Pseudo-class names are case-insensitive, e.g. `:HAS()` will work as `:has()`. +### Limitations + +1. CSS [comments](https://developer.mozilla.org/en-US/docs/Web/CSS/Comments) and [at-rules](https://developer.mozilla.org/en-US/docs/Web/CSS/At-rule) are not supported. + +2. Specific pseudo-class might have its own limitations: +[`:has()`](#extended-css-has-limitations), [`:xpath()`](#extended-css-xpath-limitations), [`:is()`](#extended-css-is-limitations), [`:not()`](#extended-css-not-limitations), and [`:remove()`](#extended-css-remove-limitations). + + ### Pseudo-class `:has()` Draft CSS 4.0 specification describes [pseudo-class `:has`](https://www.w3.org/TR/selectors-4/#relational). Unfortunately, it is not yet widely [supported by browsers](https://developer.mozilla.org/en-US/docs/Web/CSS/:has#browser_compatibility). > Synonyms `:-abp-has` and `:if` are supported for better compatibility. -> Usage of `:has()` pseudo-class is [restricted for some cases](https://bugs.chromium.org/p/chromium/issues/detail?id=669058#c54): -> 1. Disallow `:has()`, `:is()`, `:where()` inside `:has()` argument to avoid increasing the :has() invalidation complexity. -> 2. Disallow `:has()` inside the pseudos accepting only compound selectors. -> 3. Disallow `:has()` after regular pseudo-elements. - **Syntax** ``` @@ -59,6 +61,13 @@ Draft CSS 4.0 specification describes [pseudo-class `:has`](https://www.w3.org/T Pseudo-class `:has()` selects the `target` elements that includes the elements that fit to the `selector`. Also `selector` can start with a combinator. Selector list can be set in `selector` as well. + **Limitations** + +> Usage of `:has()` pseudo-class is [restricted for some cases](https://bugs.chromium.org/p/chromium/issues/detail?id=669058#c54): +> 1. Disallow `:has()`, `:is()`, `:where()` inside `:has()` argument to avoid increasing the :has() invalidation complexity. +> 2. Disallow `:has()` inside the pseudos accepting only compound selectors. +> 3. Disallow `:has()` after regular pseudo-elements. + **Examples** `div:has(.banner)` will select all `div` elements, which **includes** an element with the `banner` class: @@ -325,6 +334,8 @@ Pseudo-class `:xpath()` allows to select an element by evaluating a XPath expres - `target`- optional, standard or extended css selector - `expression` — required, valid XPath expression + **Limitations** + > `target` can be omitted so it is optional. For any other pseudo-class that would mean "apply to *all* DOM nodes", but in case of `:xpath()` it just means "apply to the *whole* document", and such applying slows elements selecting significantly. That's why rules like `#?#:xpath(expression)` are limited for looking inside the `body` tag. For example, rule `#?#:xpath(//div[@data-st-area=\'Advert\'])` is parsed as `#?#body:xpath(//div[@data-st-area=\'Advert\'])`. > Extended selectors with defined `target` as *any* selector — `*:xpath(expression)` — can still be used but it is not recommended, so `target` should be specified instead. @@ -431,8 +442,6 @@ For such DOM: Sometimes, it is necessary to remove a matching element instead of hiding it or applying custom styles. In order to do it, you can use pseudo-class `:remove()` as well as pseudo-property `remove`. -> **Pseudo-class `:remove()` is limited to work properly only at the end of selector.** - **Syntax** ``` @@ -444,8 +453,14 @@ selector { remove: true; } ``` - `selector` — required, standard or extended css selector + **Limitations** + +> Pseudo-class `:remove()` is limited to work properly only at the end of selector. + > For applying `:remove()` pseudo-class to any element [universal selector](https://www.w3.org/TR/selectors-4/#the-universal-selector) `*` should be used. Otherwise extended selector may be considered as invalid, e.g. `.banner > :remove()` is not valid for removing any child element of `banner` class element, so it should look like `.banner > *:remove()`. +> If `:remove()` pseudo-class or `remove` pseudo-property is used, all style properties will be ignored except of [`debug` pseudo-property](#selectors-debug-mode). + **Examples** ``` @@ -456,8 +471,6 @@ div:contains(advertisement) { remove: true; } div[class]:has(> a > img) { remove: true; } ``` -> If `:remove()` pseudo-class or `remove` pseudo-property is used, all style properties will be ignored except of [`debug` pseudo-property](#selectors-debug-mode). - > Rules with `remove` pseudo-property should use `#$?#` marker: `$` for CSS style rules syntax, `?` for ExtendedCss syntax. @@ -473,6 +486,8 @@ Pseudo-class `:is()` allows to match any element that can be selected by any of - `target` — optional, standard or extended css selector, can be missed for checking *any* element - `selectors` — [*forgiving selector list*](https://drafts.csswg.org/selectors-4/#typedef-forgiving-selector-list) of standard or extended selectors + **Limitations** + > If `target` is not defined or defined as [universal selector](https://www.w3.org/TR/selectors-4/#the-universal-selector) `*`, pseudo-class `:is()` applying will be limited to `html` children, e.g. rules `#?#:is(...)` and `#?#*:is(...)` are parsed as `#?#html *:is(...)`. **Examples** @@ -502,6 +517,8 @@ Pseudo-class `:not()` allows to select elements which are *not matched* by selec - `target` — optional, standard or extended css selector, can be missed for checking *any* element - `selectors` — selector list of standard or extended selectors + **Limitations** + > If `target` is not defined or defined as [universal selector](https://www.w3.org/TR/selectors-4/#the-universal-selector) `*`, pseudo-class `:not()` applying will be limited to `html` children, e.g. rules `#?#:not(...)` and `#?#*:not(...)` are parsed as `#?#html *:not(...)`. > Inside [`:upward()` pseudo-class](#extended-css-upward) argument `:not()` is considered as a standard CSS pseudo-class because `:upward()` supports only standard selectors. diff --git a/src/extended-css/extended-css.ts b/src/extended-css/extended-css.ts index ff17126e..24659a09 100644 --- a/src/extended-css/extended-css.ts +++ b/src/extended-css/extended-css.ts @@ -30,17 +30,62 @@ type ValidationResult = { error: string | null, }; +/** + * Interface for ExtendedCss constructor argument. Needed to create the instance of ExtendedCss. + * + * ExtendedCss configuration should contain: + * - CSS stylesheet of rules to apply + * - the callback for matched elements + * and also may contain a flag for global logging which is useful for selectors debugging. + * + * Stylesheet can contain not just extended selectors, and all of them will be applied by the lib. + * Stylesheet does not support CSS comments and at-rules. + */ export interface ExtCssConfiguration { - // css stylesheet + /** + * Standard CSS stylesheet — a set of CSS rules + * which generally consists of selector and style (except `:remove()` pseudo-class). + * + * ExtendedCss is able to parse and apply almost any CSS stylesheet, not just extended selectors + * but there are some limitations - for example, CSS comments and at-rules are not supported; + * learn more about the Limitations in README.md + */ styleSheet: string; - // the callback that handles affected elements + + /** + * The callback that handles affected elements. + * + * Needed for getting affected node elements and handle style properties + * before they are applied to them if it is necessary. + * + * Used by AdGuard Browser extension to display rules in Filtering log + * and `collect-hits-count` (via tsurlfilter's CssHitsCounter) + */ beforeStyleApplied?: BeforeStyleAppliedCallback; - // flag for applied selectors logging + + /** + * Optional flag for global debugging mode. + * + * Alternatively can be set by extended pseudo-property `debug: global` in styleSheet rules + * + * Learn more about Selectors debug mode in README.md + */ debug?: boolean; } /** - * Main ExtendedCss class + * Main class of ExtendedCss lib. + * + * Parses css stylesheet with any selectors (passed to its argument as styleSheet), + * and guarantee its applying as mutation observer is used to prevent the restyling of needed elements by other scripts. + * This style protection is limited to 50 times to avoid infinite loop (MAX_STYLE_PROTECTION_COUNT). + * Our own AsyncWrapper is used for styles applying to avoid too often lib reactions on page mutations. + * + * Constructor creates the instance of class which should be run be `apply()` method to apply the rules, + * and the applying can be stopped by `dispose()`. + * + * Can be used to select page elements by selector with `query()` method (similar to `Document.querySelectorAll()`), + * which does not require instance creating */ export class ExtendedCss { private context: Context; diff --git a/src/extended-css/helpers/types.ts b/src/extended-css/helpers/types.ts index ee2293c5..86603075 100644 --- a/src/extended-css/helpers/types.ts +++ b/src/extended-css/helpers/types.ts @@ -59,6 +59,9 @@ interface RemovalsStatistic { */ export type BeforeStyleAppliedCallback = (x:IAffectedElement) => AffectedElement; +/** + * Interface for ExtendedCss context. Needed to store affected elements, collect removal stats, etc. + */ export interface Context { /** * Callback that handles affected elements diff --git a/src/selector/parser.ts b/src/selector/parser.ts index f3b216ef..e38be105 100644 --- a/src/selector/parser.ts +++ b/src/selector/parser.ts @@ -49,11 +49,11 @@ import { NTH_ANCESTOR_PSEUDO_CLASS_MARKER, } from '../common/constants'; -// limit applying of wildcard :is and :not pseudo-class only to html children +// limit applying of wildcard :is() and :not() pseudo-class only to html children // e.g. ':is(.page, .main) > .banner' or '*:not(span):not(p)' const IS_OR_NOT_PSEUDO_SELECTING_ROOT = `html ${ASTERISK}`; -// limit applying of :xpath pseudo-class with to 'any' element +// limit applying of :xpath() pseudo-class to 'any' element // https://github.com/AdguardTeam/ExtendedCss/issues/115 const XPATH_PSEUDO_SELECTING_ROOT = 'body'; @@ -84,6 +84,9 @@ const doesRegularContinueAfterSpace = (nextTokenType: string, nextTokenValue: st || nextTokenValue === BRACKETS.SQUARE.LEFT; }; +/** + * Interface for selector parser context + */ interface Context { /** * Collected result diff --git a/src/selector/utils/absolute-matcher.ts b/src/selector/utils/absolute-matcher.ts index 2ebdef66..eee463c8 100644 --- a/src/selector/utils/absolute-matcher.ts +++ b/src/selector/utils/absolute-matcher.ts @@ -31,11 +31,19 @@ const REGEXP_ANY_SYMBOL = '.*'; const REGEXP_WITH_FLAGS_REGEXP = /^\s*\/.*\/[gmisuy]*\s*$/; export interface MatcherArgsInterface { - // extended pseudo-class name + /** + * extended pseudo-class name + */ pseudoName: string; - // extended pseudo-class arg + + /** + * extended pseudo-class arg + */ pseudoArg: string; - // dom element to check + + /** + * dom element to check + */ domElement: Element; } @@ -179,7 +187,7 @@ interface PseudoArgData { * for :matches-attr() and :matches-property() - with EQUAL_SIGN as separator * @param pseudoArg * @param separator - * @returns {PseudoArgData} { name, value } where 'value' can be undefined + * @returns {PseudoArgData} */ const getPseudoArgData = (pseudoArg: string, separator: string): PseudoArgData => { const index = pseudoArg.indexOf(separator); diff --git a/src/selector/utils/query-helpers.ts b/src/selector/utils/query-helpers.ts index 481be67e..11a95484 100644 --- a/src/selector/utils/query-helpers.ts +++ b/src/selector/utils/query-helpers.ts @@ -44,15 +44,29 @@ import { logger } from '../../common/utils/logger'; */ export type Specificity = string; -export interface RelativePredicateArgsInterface { - // dom element to check relatives +/** + * Interface for relative pseudo-class helpers args + */ +interface RelativePredicateArgsInterface { + /** + * dom element to check relatives + */ element: HTMLElement; - // SelectorList node + + /** + * SelectorList node + */ relativeSelectorList: AnySelectorNodeInterface; - // extended pseudo-class name + + /** + * extended pseudo-class name + */ pseudoName: string; - // flag for error throwing on invalid selector from selectorList - // e.g. true for :not() pseudo-class + + /** + * flag for error throwing on invalid selector from selectorList + * e.g. true for :not() pseudo-class + */ errorOnInvalidSelector?: boolean; } diff --git a/src/stylesheet/parser.ts b/src/stylesheet/parser.ts index 37ed6d3d..0ce40a1c 100644 --- a/src/stylesheet/parser.ts +++ b/src/stylesheet/parser.ts @@ -47,23 +47,71 @@ export interface CssStyleMap { [key: string]: string; } +/** + * Interface for rules data parsed from passed styleSheet + */ export interface ExtCssRuleData { + /** + * selector text + */ selector: string; + + /** + * selector ast to query dom elements + */ ast: AnySelectorNodeInterface; + + /** + * styles to apply to matched dom elements + */ style?: CssStyleMap; + + /** + * flag for specific rule debugging mode + */ debug?: string; + + /** + * log data, available only for debugging mode + */ timingStats?: TimingStats; + + /** + * dom elements matched by rule, available only for debugging mode + */ matchedElements?: HTMLElement[]; } +/** + * Interface for storing data parsed from selector rule part + */ interface SelectorPartData { + /** + * success status + */ success: boolean; + + /** + * parsed selector + */ selector: string; - // might be not defined if selector is not valid + + /** + * selector ast to query elements by, + * might be not defined if selector is not valid + */ ast?: AnySelectorNodeInterface; + + /** + * styles parsed from selector rule part, + * relevant to rules with `:remove()` pseudo-class which may not have actual style declaration + */ stylesOfSelector?: Style[]; } +/** + * Interface for stylesheet parser context + */ interface Context { /** * Flag for parsing rules parts diff --git a/test/helpers/selector-parser.ts b/test/helpers/selector-parser.ts index 3c1498c3..5fa21c83 100644 --- a/test/helpers/selector-parser.ts +++ b/test/helpers/selector-parser.ts @@ -24,7 +24,7 @@ export const getRegularSelector = (regularValue: string): TestAnySelectorNodeInt /** * Returns extended selector AbsolutePseudoClass node * @param name extended pseudo-class name - * @param arg arg of pseudo-class + * @param value value of pseudo-class */ export const getAbsoluteExtendedSelector = (name: string, value: string): TestAnySelectorNodeInterface => { return { @@ -113,8 +113,7 @@ export const getAstWithSingleRegularSelector = (regularValue: string): TestAnySe * 1. Only one of isRegular/isAbsolute/isRelative should be true * 2. Acceptable parameters for: * - isRegular: value - * - isAbsolute: name, arg - * - isRelative: name, value + * - isAbsolute or isRelative: name, value */ interface AnyChildOfSelectorRaw { isRegular?: boolean, @@ -122,7 +121,6 @@ interface AnyChildOfSelectorRaw { isRelative?: boolean value?: string, name?: string, - arg?: string, } /** @@ -171,9 +169,17 @@ export const getSingleSelectorAstWithAnyChildren = ( }; interface SelectorListOfRegularsInput { - actual: string, // selector for parsing - expected: string[], // array of expected values for RegularSelector nodes + /** + * selector for parsing + */ + actual: string, + + /** + * array of expected values for RegularSelector nodes + */ + expected: string[], } + /** * Checks whether the passed selector is parsed into proper SelectorList * @param input - { actual, expected } @@ -183,9 +189,17 @@ export const expectSelectorListOfRegularSelectors = ({ actual, expected }: Selec }; interface SelectorListOfAnyChildrenInput { - actual: string, // selector for parsing - expected: AnyChildOfSelectorRaw[] // array of data for building ast + /** + * selector for parsing + */ + actual: string, + + /** + * array of data for building ast + */ + expected: AnyChildOfSelectorRaw[], } + /** * Checks whether the 'actual' is parsed into AST with specified parameters * @param input - { actual, expected } @@ -195,9 +209,17 @@ export const expectSingleSelectorAstWithAnyChildren = ({ actual, expected }: Sel }; interface ToThrowSelectorInput { - selector: string, // selector for extCss querySelectorAll() - error: string, // error text to match + /** + * selector for extCss querySelectorAll() + */ + selector: string, + + /** + * error text to match + */ + error: string, } + export const expectToThrowInput = (input: ToThrowSelectorInput): void => { const { selector, error } = input; expect(() => { diff --git a/test/helpers/selector-query-jsdom.ts b/test/helpers/selector-query-jsdom.ts index 538cc5db..6a52d43b 100644 --- a/test/helpers/selector-query-jsdom.ts +++ b/test/helpers/selector-query-jsdom.ts @@ -21,8 +21,15 @@ export const expectNoMatch = (selectedElements: HTMLElement[]) => { }; interface SuccessSelectorInput { - actual: string, // selector for extCss querySelectorAll() - expected: string, // target selector for checking + /** + * selector for extCss querySelectorAll() + */ + actual: string, + + /** + * target selector for checking + */ + expected: string, } export const expectSuccessInput = (input: SuccessSelectorInput): void => { const { actual, expected } = input; @@ -33,7 +40,10 @@ export const expectSuccessInput = (input: SuccessSelectorInput): void => { }; interface NoMatchSelectorInput { - selector: string, // selector for extCss querySelectorAll() + /** + * selector for extCss querySelectorAll() + */ + selector: string, } export const expectNoMatchInput = (input: NoMatchSelectorInput): void => { const { selector } = input; @@ -43,8 +53,15 @@ export const expectNoMatchInput = (input: NoMatchSelectorInput): void => { }; interface ToThrowSelectorInput { - selector: string, // selector for extCss querySelectorAll() - error: string, // error text to match + /** + * selector for extCss querySelectorAll() + */ + selector: string, + + /** + * error text to match + */ + error: string, } export const expectToThrowInput = (input: ToThrowSelectorInput): void => { const { selector, error } = input;