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

Implement :not() pseudo selector #3256

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tmolitor-stud-tu
Copy link

I'm not sure if this complys wih your coding guidlines and it might not be the way you'd like it be solved, but maybe it's still useful :)

Copy link
Member

@bsweeney bsweeney left a comment

Choose a reason for hiding this comment

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

I look forward to reviewing this change.

src/Css/Stylesheet.php Outdated Show resolved Hide resolved
@bsweeney bsweeney added this to the 2.0.5 milestone Sep 7, 2023
This may be a dirty hack, but it seems to work well.
@tmolitor-stud-tu
Copy link
Author

Anything missing here?

@bsweeney
Copy link
Member

I have not yet reviewed this change. I'm thinking now I'll take a look at inclusion for the next release.

@bsweeney bsweeney modified the milestones: 2.1.1, 2.1.0 Nov 24, 2023
Comment on lines +658 to +660
if(substr($subquery, 0, 4) != '//*[') {
return null; //not supported
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we can build out support based on the logic we have for the matches selector.

$length = strpos($selector, ")", $i) - $p;
$i += $length + 1;
$matchList = trim(mb_substr($selector, $p, $length));
$subquery = $this->selectorToXpath($matchList, true)['query'];
Copy link
Member

Choose a reason for hiding this comment

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

The subquery produced if $matchList is a comma-delimited list is broken. I think we need to split the string and parse each selector individually, similar to what's happening for matches.

Comment on lines +657 to +661
$subquery = $this->selectorToXpath($matchList, true)['query'];
if(substr($subquery, 0, 4) != '//*[') {
return null; //not supported
}
$query .= "[not(".substr($subquery, 4, -1).")]";
Copy link
Member

@bsweeney bsweeney Feb 2, 2024

Choose a reason for hiding this comment

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

Recursing into selectorToXpath is problematic since we don't want a full XPath query. That being said the following small tweak seems to work better, though it's still not great. It supports both elements and attributes, but combined selectors are still problematic (e.g., element.class or .class1.class2).

Suggested change
$subquery = $this->selectorToXpath($matchList, true)['query'];
if(substr($subquery, 0, 4) != '//*[') {
return null; //not supported
}
$query .= "[not(".substr($subquery, 4, -1).")]";
$subqueries = array_map("trim", explode(",", $matchList));
foreach ($subqueries as &$subquery) {
if (preg_match("/^\w/", $subquery)) {
$subquery = "name() = '$subquery'";
} else {
$subquery = substr($this->selectorToXpath($subquery, true)['query'], 4, -1);
}
}
$query .= "[not(" . implode(" or ", $subqueries) . ")]";

@bsweeney bsweeney linked an issue Feb 10, 2024 that may be closed by this pull request
@bsweeney bsweeney modified the milestones: 3.0.0, 3.1.0 Feb 10, 2024
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.

Add support for the :not() psuedo-selector
2 participants