andybalholm / cascadia

CSS selector library in Go
BSD 2-Clause "Simplified" License
703 stars 65 forks source link

Add support for pseudo elements #40

Closed benoitkugler closed 5 years ago

benoitkugler commented 5 years ago

Hi again, This PR adds support for pseudo elements.

The approach taken boils down to adding a method PseudoElement on Sel, even for simple selectors, where the implementation is "empty".

andybalholm commented 5 years ago

Somehow we need to make sure that a pseudo-element selector doesn't match anything at all unless the caller explicitly supports pseudo-elements.

Would it be better to have a MatchPseudoElement(n *html.Node) (match bool, pseudo string)? It would return true, "" on a non-pseudo match and true, "first-line" (or whatever) on a pseudo match.

benoitkugler commented 5 years ago

Why not, but I'm not sure to understand the need for this, since pseudo-elements can't change the node selected. They are only linked to a selector, and can be retrieved on matching, but don't have any influence on the matching (specificity aside). Or am I missing something ?

andybalholm commented 5 years ago

A selector like button::before does not match any real elements. It only matches pseudo elements. Try entering document.querySelectorAll("button::before") in the developer console for this page. You'll see that it returns an empty node list, even though there are buttons on the page.

benoitkugler commented 5 years ago

Ah, you're right ! Well, from what I understand, it's up to the user to validate the pseudo-element, that's why a browser won't give you anything for an invalid pseudo-element.

andybalholm commented 5 years ago

If somebody enters a selector with a pseudo-element into a program that doesn't support pseudo-elements, they should either get an error, or it should do nothing. So we need to come up with an API that works this way for programs that don't specifically ask for pseudo-element support.

benoitkugler commented 5 years ago

In this case, we could add a boolean parameter withPseudoElements to Parse() : when false, parsing a pseudo-element return an error; when true it behaves like in this PR (that is "button::before" match every button). The Compile() function could call Parse() with withPseudoElements = false to maintain backward compatibility.

andybalholm commented 5 years ago

I think you're on the right track. But I don't want to clutter up the common use case (no pseudo elements) with an extra parameter. I suppose that means we'd need to add a ParseWithPseudoElements method or something.

I noticed you're working on porting a project from Python. Does it use a general-purpose selector library? If so, how does it handle this issue?

benoitkugler commented 5 years ago

They have built their own selector library cssselect2. Actually, this PR is directly inspired from their API : for example, the selector button::before applied on the html <button></button>, does match, returning the specificity and the pseudo element before. I guess the support of pseudo elements was a goal from the beginning.

benoitkugler commented 5 years ago

So, as you suggested, I added a method ParseGroupWithPseudoElements. The default behavior for Parse and ParseGroup is left unchanged : selector with pseudo-element error at parse time.

andybalholm commented 5 years ago

Your doc comment mentions ParseWithPseudoElements, but you didn't implement that, just ParseGroupWithPseudoElements.

benoitkugler commented 5 years ago

Added ParseWithPseudoElement alongside ParseGroupWithPseudoElements