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 CSS pseudo elements - use CSSPseudoElement interface #40

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

milahu
Copy link
Contributor

@milahu milahu commented Sep 12, 2020

use the CSSPseudoElement interface
https://drafts.csswg.org/css-pseudo-4/#csspseudoelement

interface CSSPseudoElement : EventTarget {
    readonly attribute CSSOMString type;
    readonly attribute Element element;
};

so in jsdom we can avoid workarounds like special e.nodeType values

e.type includes colons, like :before or ::before

Boolean(e.element)

has the highest selectivity
only pseudo elements have the element attribute
proof:

$ grep -E 'attribute .* element;' *.webidl
CSSPseudoElement.webidl:    readonly attribute Element element;

i expect when e.element is set, then also e.type is set

browser support for the CSSPseudoElement interface is virtually zero
only firefox has it, when in about:config you set
dom.css_pseudo_element.enabled = true
(not working for me)
https://developer.mozilla.org/en-US/docs/Web/API/CSSPseudoElement
https://developer.mozilla.org/en-US/docs/Web/API/CSSPseudoElement/element#Examples

@dperini
Copy link
Owner

dperini commented Jun 23, 2021

@milahu
I am in the process of adding pending features and fixes for the next new release.
the only doubt I have is the removal/substitution of the "D" values in both resolvers:

if(' + D + '(e.nodeType==1))

Even if I am sure you have complete control and understanding of the code, could you please double-check and confirm ?
The D flag assumes its value from the last parameter passed in to the "compileSelector()" function for when we need to negate the behavior/result of the "compileSelector()".

Atop the "compileSelector()" function there are two lines describing both the "N" and "D" variables.

  // N is the negation pseudo-class flag
  // D is the default inverted negation flag

Sorry for the delay implementing this.

@milahu
Copy link
Contributor Author

milahu commented Jun 23, 2021

its been a while ; )

when we need to negate the behavior/result of the "compileSelector()".

makes no sense in the context of pseudo elements (does it?)

this patch depends on my patch for jsdom (just a working prototype ...)

snippet 1

    // CSSPseudoElement
    // TODO
    // workaround for nwsapi resolver functions
    // who expect (!(e.nodeType == 1))
    // for pseudo elements
    this.nodeType = -1;

snippet 2

  get element() {
    return this._parentElement;
  }
  get type() {
    return '::'+this._pseudoId;
  }

i guess the D just looked constant to me ...

compileSelector is always called with non = false (is it?)
so D = not ? '' : '!' is always '!'

source = compileSelector(selector, macro, mode, callback, false);

i moved from node.nodeType = -1 (magic value) to node.element and node.type
since that is the official CSSPseudoElement interface

@dperini
Copy link
Owner

dperini commented Jun 23, 2021

@milahu
you are correct the "compileSelector( ... )" signature changed making the last parameter obsolete (always false).
So this would be an extra optimization to do, the last parameter and "D", "N" variables can be removed completely.
Before "compileSelector( ... )" was invoked recursively, but now we use internal "match()" or "!match()" to achieve that.
Sorry for the delayed commits there was no time left for me for this in the last year. Hope things are getting better now.
I am going to merge the pending requests.

@dperini dperini merged commit ad64015 into dperini:master Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants