unescape css identifiers to allow use in css queries #1303
Conversation
|
@flavorjones Any feedback on this? Had this issue reported to Capybara when a user was trying to select elements with '.' in their ids using a css id selector. |
|
|
||
|
|
||
| def unescape_css_identifier(identifier) | ||
| identifier.gsub(/\\(?:([^0-9a-fA-F])|([0-9a-fA-F]{1,6})\s?)/){ |m| $1 || [$2.hex].pack('U') } |
There was a problem hiding this comment.
Can you provide some context around this regex? Did you pull it from an RFC?
There was a problem hiding this comment.
it matches a \ followed by any character other than hex characters, so : , etc - or any 1-6 digit hex number potentially followed by whitespace - these are the two methods of escaping characters in css ids, classes, etc
There was a problem hiding this comment.
and from http://www.w3.org/TR/css3-selectors/#selector-syntax
"Characters in Selectors can be escaped with a backslash according to the same escaping rules as CSS. [CSS21]"
twalpole
force-pushed
the
css_identifier_unescape
branch
from
82a63c8 to
9e0751c
Compare
|
@twalpole - Ping. Saw your email on nokogiri-talk. How can I help? |
|
@flavorjones What changes are needed for this PR to be accepted? - the current test failures are only The ones caused by the latest commit on master |
|
@twalpole It sounds like you're asking why this wasn't nominated for inclusion in 1.6.7. A few reasons:
These things combined led me to move it slightly lower on the priority list. That said, I'd like to do a fast-follow of 1.6.8 to get a bunch of bugfixes in, so I'm marking this for 1.6.8 and will make a point of getting it out as quickly as I reasonably can. Thanks for your patience. |
|
Thanks for the explanation/status update - I really just wanted to know whether it was acceptable , needed changes, or I needed to consider trying to somehow patch it from capybara. I'll wait for 1.6.8 to start firming up |
|
@twalpole -Thanks for following up. I would like one more test added, to more accurately reflect the underlying use case at teamcapybara/capybara#1541, which is the ability to query for an encoded css attribute. Would you mind adding such a test? The unit tests written so far simply assert on the xpath generated from an escaped query, and not that a properly-unescaped query will actually return the expected nodeset. |
|
@flavorjones test added, would you like me to squash them down to one commit? |
|
@twalpole Thanks so much for doing that! LGTM, will be in 1.6.8 when it drops. |
|
Merged onto master! Thanks again, will be in 1.6.8. |
Allow for querying css that uses "special" characters in id and class names such as
querying with
document.css('#a.b')
will work correctly with this PR - failed previously