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

[stoivo]

I am getting warings when I use roadie when I want inline styles. It loops over the stylessheet and inlines the styles for each selector. It also inlines the styles from normalize.css

button:-moz-focusring,
[type="button"]:-moz-focusring,
[type="reset"]:-moz-focusring,
[type="submit"]:-moz-focusring {
  outline: 1px dotted ButtonText;
}

I created a little sample script to demonstrate the issue.

require 'nokogiri'
doc = Nokogiri.XML('<foo></foo>')

p doc.send(:css_rules_to_xpath, ['a:hover'], {})
p doc.css('a:hover')

p doc.send(:css_rules_to_xpath, ['div:empty'], {})
p doc.css('div:empty')

p doc.send(:css_rules_to_xpath, ['a:a-moz-drag-over'], {})
p doc.css('a:a-moz-drag-over')

p doc.send(:css_rules_to_xpath, ['a:-moz-drag-over'], {})
p doc.css('a:-moz-drag-over')

output

["//a[nokogiri:hover(.)]"] [] ["//div[not(node())]"] [] ["//a[nokogiri:a-moz-drag-over(.)]"] [] ["//a[nokogiri:-moz-drag-over(.)]"] /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/xml/searchable.rb:238:in `evaluate': ERROR: Invalid expression: //a[nokogiri:-moz-drag-over(.)] (Nokogiri::XML::XPath::SyntaxError) from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/xml/searchable.rb:238:in `xpath_impl' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/xml/searchable.rb:219:in `xpath_internal' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/xml/searchable.rb:211:in `css_internal' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/xml/searchable.rb:132:in `css' from /Users/simon/dev/work/dodo/fakturabank/repo/bb.rb:28:in `<main>' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/commands/runner/runner_command.rb:43:in `load' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/commands/runner/runner_command.rb:43:in `perform' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor/command.rb:27:in `run' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor/invocation.rb:127:in `invoke_command' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor.rb:392:in `dispatch' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/command/base.rb:87:in `perform' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/command.rb:48:in `invoke' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/commands.rb:18:in `<main>' from <internal:/Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require' from <internal:/Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require' from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require' from bin/rails:4:in `<main>'

As you can see it don't crash for a-moz-drag-over which is an unknown pseudo class. Maybe it crashed because it starts on a -. Is this something we can fix or should I try to fix it on the outside?

[flavorjones]

@stoivo Thank you for opening this issue!

This error is coming from libxml2 (and not from Nokogiri), but let me investigate and see if we can work around it.

[flavorjones]

@stoivo Can you tell me more about your use case? I'm not familiar with roadie.

The :-moz-drag-over pseudoclass is not something that can be implemented without a browser, and so will never match anything. Why are you (or why is roadie) doing a search on the document that you know ahead of time will not return any matches?

I just want to make sure I understand the use case before getting too far into solving this.

[stoivo]

yeah, roadie help to inline styles for mails, since that recommended way to using css in mails.

We take application.css and inlines those rules. It's done by parsing the css stylesheet and looping over css rule by css rule and searching for the element by css. in our application.css we have included normalize.css which has -moz-focusring rules. We could solve this in roadi too by not searching if the rule ends with -moz-focusring

[flavorjones]

@stoivo Thanks for giving additional context.

I read through the relevant portions of the XPath query spec. For context, pseudo-classes are implemented as function calls in Nokogir's CSS-to-XPath translator, and libxml2 has implemented the XPath's spec for parsing function names properly.

Unfortunately, in XPath, function names are required to start with a NameStartChar which does not include the - character. There's nothing we can easily do to change that, and I don't think it's beneficial enough to warrant the work at this time.

For now, your best bet is to rescue Nokogiri::XML::XPath::SyntaxError and skip to the next search. Sorry I couldn't be more helpful!

[flavorjones]

Hmm, actually, please tell me if there's a way Nokogiri can signal this type of error to users earlier that would be helpful for you?

It might make sense to have the XPath translation raise an error (and not the search). Would that be helpful?

[flavorjones]

@stoivo I'm curious how the branch at #3197 works for you?

[stoivo]

I didn't test the branch. It seam nice to have a more descriptive error

We have css sheets with selectors which are invalid like div:not(.one which we want to warn the user about. This selector also raises Nokogiri::CSS::SyntaxError which is good. I think it fine that we add a small check to do nothing if the function starts with -.

I did a bit more testing and it seam like the CSS parser don't like "div:not(.one,.two)"

parser = Nokogiri::CSS::Parser.new
pp parser.parse("div:not(.one,.two)")

__END__
/Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/css/parser_extras.rb:86:in `on_error': unexpected ',' after '#<Nokogiri::CSS::Node:0x0000000117852470>' (Nokogiri::CSS::SyntaxError)
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/racc-1.7.3/lib/racc/parser.rb:267:in `_racc_do_parse_c'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/racc-1.7.3/lib/racc/parser.rb:267:in `do_parse'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/nokogiri-1.16.5-arm64-darwin/lib/nokogiri/css/parser_extras.rb:68:in `parse'
  from /Users/simon/dev/work/dodo/fakturabank/repo/bb.rb:36:in `<main>'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/commands/runner/runner_command.rb:43:in `load'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/commands/runner/runner_command.rb:43:in `perform'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor/command.rb:27:in `run'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor/invocation.rb:127:in `invoke_command'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/thor-1.2.2/lib/thor.rb:392:in `dispatch'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/command/base.rb:87:in `perform'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/command.rb:48:in `invoke'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/railties-7.0.8/lib/rails/commands.rb:18:in `<main>'
  from <internal:/Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  from <internal:/Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  from /Users/simon/.asdf/installs/ruby/3.2.1/lib/ruby/gems/3.2.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32:in `require'
  from bin/rails:4:in `<main>'

[flavorjones]

I did a bit more testing and it seam like the CSS parser don't like "div:not(.one,.two)"

Sorry, is this comment related to the original issue or the pull request fixing it? If you've found another bug, please open a new issue.

I would really appreciate you trying this branch out. I'm trying to be responsive to the issue you filed.