Sizzle regex adjustment #22581
Sizzle regex adjustment #22581
Conversation
In cockpit-machines there are selectors like `.pf-v6-c-menu button:contains('Serial (serial0)')`,
the previous regex did not match brackets.
To support cockpit-machines which has a selector after :contains()
such as for example:
```
b.click(f"#os-select li:contains('{dialog.os_name}') button")
```
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (!window.Sizzle) { | ||
| // Best effort support `:contains()` | ||
| const re = /:contains\(([\s\S]*?)\)/g; | ||
| const re = /:contains\(((?:[^()]*|\([^()]*\))*)\)/g; |
Check failure
Code scanning / CodeQL
Inefficient regular expression High test
There was a problem hiding this comment.
Yeah only thing here is that it will have exponential backtrack when we're missing a ).
.pf-v6-c-menu__item:contains('SpiceOnly' input:checked
There was a problem hiding this comment.
Aha, so I guess not really a problem for us. Honestly not a regex expert so welcome to improvements.
There was a problem hiding this comment.
Is it required to have quotes for :contains()? If so that will make it a lot easier as we can check for start and end better
There was a problem hiding this comment.
It would be, currently we allow non-quotes. If we want to enforce quotes we need to accept both '/" ofcourse and have a way to mass convert tests where needed.
There was a problem hiding this comment.
As a middle ground: could we continue to allow simple strings without quotes, and anything which contains parentheses etc. has to be quoted? Something like /:contains\( ([a-zA-Z0-9]+) | '([^']+)' \) (of course without the extra spaces, just to make it a bit more readable)? If you care, we could add another alternative for double quotes of course.
| return elem_contains_text(elems, searchText); | ||
| // everything after :contains() | ||
| const leftover = sel.substring(matches.index + matches[1].length + ":contains()".length, sel.length).trim(); | ||
| const base = sel.replace(re, '').replace(leftover, ''); |
There was a problem hiding this comment.
This feels brittle. I'd prefer if you could compute the index after :contains() once, and split the string into base and leftover at that point.
See cockpit-project#22581 (review) This commit was done entirely with Claude Code: > in test/, find all instances of `:contains` selectors with an unquoted value, like > b.click("#account-expiration .pf-v6-c-modal-box__footer button:contains(Change)") > quote all these; prefer `'` unless it collides with the outer quoting
See cockpit-project#22581 (review) This commit was done entirely with Claude Code: > in test/, find all instances of `:contains` selectors with an unquoted value, like > b.click("#account-expiration .pf-v6-c-modal-box__footer button:contains(Change)") > quote all these; prefer `'` unless it collides with the outer quoting
See #22581 (review) This commit was done entirely with Claude Code: > in test/, find all instances of `:contains` selectors with an unquoted value, like > b.click("#account-expiration .pf-v6-c-modal-box__footer button:contains(Change)") > quote all these; prefer `'` unless it collides with the outer quoting
3 participants
This PR does two things to support cockpit-machines:
.pf-v6-c-menu__item:contains('SpiceOnly') input:checkedThe changes are run here cockpit-project/cockpit-machines#2387