[Autocomplete] Escape querySelector dynamic selector with CSS.escape()
#2663
Conversation
carsonbot
added
Autocomplete
Bug
📊 Packages dist files size differenceThanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
|
|||||||||
|
Should I worry about the 3 failed checks? |
| @@ -325,6 +325,10 @@ export default class extends Controller { | |||
| return string.replace(/(<([^>]+)>)/gi, ''); | |||
| } | |||
|
|
|||
| #addSlashes(string: string): string { | |||
There was a problem hiding this comment.
I recently learned the existence of CSS.escape() Would it not be better than a custom implementation here ?
There was a problem hiding this comment.
CSS.escape escapes more then strictly necessary but the solution you suggest is cleaner indeed.
| @@ -192,7 +192,7 @@ export default class extends Controller { | |||
| for (const [, tomSelectOption] of Object.entries(this.tomSelect.options)) { | |||
| if (tomSelectOption.$order === optionOrder) { | |||
| orderedOption = parentElement.querySelector( | |||
| `:scope > option[value="${tomSelectOption[this.tomSelect.settings.valueField]}"]` | |||
| `:scope > option[value="${[this.#addSlashes(this.tomSelect.settings.valueField)]}"]` | |||
There was a problem hiding this comment.
Are we sure here ? What is the goal of providing an array ?
There was a problem hiding this comment.
Removed the array again.
Hello @Bartheyrman22 ... do you have a simple reproducer we can test to see what happens here ? |
I believe it is visible as soon as you debug in Firefox-debugger. |
|
You can start from the template i started ... it's not much more than a fresh install of symfony webapp + CSS .. but it's already a starting point :) You'll still need to install ux-autocomplete and add a new form to show the problem (probably composer update to start 😅 ) |
I don't think so... Twig/Live are 🔴 for another reason... and I just opened a PR to fix the JS files false-positive :) |
Kocal
changed the title
querySelector dynamic selector with CSS.escape()
carsonbot
added
Status: Reviewed
|
Thank you @Bartheyrman22. |
Properly escape the querySelector for option-values like "["Test"]"/
This to mitigate:
Some remark where I don't have a solution for:
The issue is only reproduceable on Chrome.
In Firefox, the code in the controller.js is not executed properly. So the error get's never triggered.
But no side effects detected so far.