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

Browser Interactivity API should escape CSS IDs when building selectors #6211

Closed
6 tasks done
zacharyvoase opened this issue Jul 24, 2024 · 3 comments · Fixed by #6243
Closed
6 tasks done

Browser Interactivity API should escape CSS IDs when building selectors #6211

zacharyvoase opened this issue Jul 24, 2024 · 3 comments · Fixed by #6243
Labels
feat: browser Issues and PRs related to the browser runner p3-minor-bug An edge case that only affects very specific usage (priority) pr welcome

Comments

@zacharyvoase
Copy link
Contributor

Describe the bug

The issue is here:

— the raw ID is inserted into the CSS selector string as-is, but that may not always produce valid CSS, and causes an error during the test.

You can reproduce this issue just by defining a <button id=":foo"> and trying to use userEvent.click() on it.

Almost any string can be a valid HTML ID (see here), but that does not mean it can be used unescaped in a CSS selector. It is possible to escape characters outside the regular range by following these instructions. One could write a small function, e.g., sanitizeHtmlIdForCss(htmlId: string): string, and just call that on el.id before passing it into the string interpolation in the line of code I linked to above.

If this seems worth addressing I can submit this PR myself, just let me know. Thanks!

Reproduction

This is hard/impossible to reproduce in Stackblitz because it only applies in the browser, and I don't think Stackblitz can run those. But if you have a local project that runs tests in the browser, try creating an element with an ID of :foo, and call any userEvent method on it.

System Info

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M1 Max
    Memory: 653.55 MB / 64.00 GB
    Shell: 3.7.0 - /opt/homebrew/bin/fish
  Binaries:
    Node: 21.7.1 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.5.0 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 126.0.6478.183
    Safari: 17.5
    Safari Technology Preview: 18.0
  npmPackages:
    @vitejs/plugin-react: ^4.2.1 => 4.2.1
    @vitest/browser: ^2.0.4 => 2.0.4
    vite: ^5.3.4 => 5.3.4
    vitest: ^2.0.4 => 2.0.4

Used Package Manager

npm

Validations

@sheremet-va sheremet-va added feat: browser Issues and PRs related to the browser runner p3-minor-bug An edge case that only affects very specific usage (priority) pr welcome and removed pending triage labels Jul 25, 2024
@ysgk
Copy link

ysgk commented Jul 27, 2024

As a side note, ids generated by React's useId() hook are wrapped with colons, like :r1:, so the current behavior can be problematic in React projects.

@AriPerkkio
Copy link
Member

I ran into this same problem with Cypress couple of months ago - exactly with useId() React hook. Maybe similar solution could work here: cypress-io/cypress#27922 (comment)

@zacharyvoase
Copy link
Contributor Author

I have also been running into issues with case-sensitivity in IDs (yes, CSS is generally case-insensitive, but this does not apply when it is referring to identifiers and class names in HTML). This issue can be replicated simply by making an HTML ID that uses uppercase letters. I am going to take some time this evening to make a PR. Does this project have regression tests that I could easily add to, to demonstrate the problem and fix?

@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: browser Issues and PRs related to the browser runner p3-minor-bug An edge case that only affects very specific usage (priority) pr welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants