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

fix: CSS queries with xpath-incompatible pseudo-classes raise an exception at query parse time #3197

Merged
merged 3 commits into from
May 24, 2024

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented May 20, 2024

What problem is this PR intended to solve?

fix: raise CSS::SyntaxError if a pseudo-class is not an XPath Name

Some pseudo-classes cannot be converted into an XPath function name, and libxml2 will raise an Nokogiri::XML::XPath::SyntaxError at query-time:

nokogiri/xml/searchable.rb:238:in `evaluate': ERROR: Invalid expression: //*:div[nokogiri:-moz-drag-over(.)] (Nokogiri::XML::XPath::SyntaxError)

This change moves the error from query-time to parse-time, in the hopes that this is more rescuable (and the error is more descriptive):

nokogiri/css/parser_extras.rb:86:in `on_error': unexpected '-' after ':' (Nokogiri::CSS::SyntaxError)

Closes #3193

Have you included adequate test coverage?

Yes.

Does this change affect the behavior of either the C or the Java implementations?

N/A

lib/nokogiri/css/tokenizer.rb Dismissed Show dismissed Hide dismissed
lib/nokogiri/css/tokenizer.rb Dismissed Show dismissed Hide dismissed
lib/nokogiri/css/tokenizer.rb Dismissed Show dismissed Hide dismissed
- use parens more intentionally in the rexical macros
- rename the CSS ID token macro from `name` to `charref` to match XPath CharRef
- extract a `name` macro that matches XPath Name
Some pseudo-classes cannot be converted into an XPath function name,
and libxml2 will raise an Nokogiri::XML::XPath::SyntaxError at
query-time:

    nokogiri/xml/searchable.rb:238:in `evaluate': ERROR: Invalid expression: //*:div[nokogiri:-moz-drag-over(.)] (Nokogiri::XML::XPath::SyntaxError)

This change moves the error from query-time to parse-time, in the
hopes that this is more rescuable (and the error is more descriptive):

    nokogiri/css/parser_extras.rb:86:in `on_error': unexpected '-' after ':' (Nokogiri::CSS::SyntaxError)

Closes #3193
@flavorjones flavorjones merged commit 1c50ff8 into main May 24, 2024
131 of 132 checks passed
@flavorjones flavorjones deleted the 3193-ncname-pseudoclass branch May 24, 2024 20:59
flavorjones added a commit that referenced this pull request Jun 11, 2024
This is an alternative to #3197 (which was reverted) in which the
exception is raised from the XPathVisitor and not the CSS
Parser.

Semantically, this is valid CSS, and so the Parser shouldn't
raise. But it is invalid XPath, and so it's the responsibility of the
Visitor to raise.

Closes #3193.
flavorjones added a commit that referenced this pull request Jun 11, 2024
**What problem is this PR intended to solve?**

After some thought, I decided I didn't like the solution to #3193
introduced in #3197, in which the exception was raised by the CSS
parser, because it's an XPath translation problem and not a CSS problem.

This PR reverts that change, and instead raises the exception from the
XPathVisitor class when it tries to translate the pseudo-class selector
or function into an XPath function call.


**Have you included adequate test coverage?**

Yes.


**Does this change affect the behavior of either the C or the Java
implementations?**

N/A
@flavorjones flavorjones added this to the v1.17.0 milestone Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] CSS selector issue -moz-drag-over nonstandard Pseudo-classes
1 participant