isTabbableRadio doesn't properly escape query selectors

[tidychips]

It's possible for radio button names to contain characters that are invalid when used in a CSS selector (e.g. "). Below is an example that causes the tabbable function to throw an error due to an invalid selector constructed at:

tabbable/src/index.js

Line 105 in cd06b98

'input[type="radio"][name="' + node.name + '"]'

HTML:

<div>
  <input type="radio" id="huey" name="&quot;duck&quot;" value="huey"
         checked>
  <label for="huey">Huey</label>
</div>

<div>
  <input type="radio" id="dewey" name="&quot;duck&quot;" value="dewey">
  <label for="dewey">Dewey</label>
</div>

JS:

import { tabbable } from 'tabbable';
// Throws DOMException: Document.querySelectorAll: 'input[type="radio"][name=""duck""]' is not a valid selector
tabbable(document.body);

It's also possible to inject values into the selector with a name like:

<input type="radio" id="dewey" name="&quot;] .some-class[data-attr=&quot;some-attribute" value="dewey">

A potential solution is CSS.escape, but it does require a polyfill for IE.

[idoros]

I'm for adding CSS.escape around the radio element name,

However I wouldn't add the polyfill, just a no-op fallback, and write about it in the docs because:

1. IE being almost at end of life.
2. IE has ~1% - ~1.89% market share at this point
3. this is not a very common name (I get that it might be used by some automated system)
4. polyfill will add ~50% to the size of tabbable

I'm not sure injection could cause any serious trouble here, the process that the query is being used in just checks if a radio element is not tabbable (notice that either way it's still a valid focusable element): (1) get a list of candidates elements that are supposed to be in the tested radio group (here you might cause different elements to be selected) (2) if one of those elements has a positive .check property and it's not the tested radio, then the originally tested radio element that might not suppose to be tabbable will return as tabbable.

[tidychips]

Thanks for the suggestion around the polyfill @idoros, I've created PR following it. I also agree with your analysis of the injection scenarios, and that the ramifications of such are fairly trivial.

[stefcameron]

@all-contributors add @tidychips for bug

[allcontributors]

@stefcameron

I've put up a pull request to add @tidychips! 🎉