Skip to content

Commit

Permalink
Escape CSS selector special characters in radio button names (#169)
Browse files Browse the repository at this point in the history 
* fix: escape radio button names in selectors
* docs: add IE polyfill caveat to README
* add changeset
* fix: update CSS.escape fallback to return argument
* docs: fix grammar
* fix: Update CSS.escape check for SSR
* fix: add related issue to changeset
* fix: Proper logic for CSS.escape polyfill with SSR
* fix: replace " characters in CSS.escape fallback
* Revert "fix: replace " characters in CSS.escape fallback"
* fix: update special character fallback behavior
* tests: Add test for CSS.escape fallback
* fix: Fix yarn lockfile
* No try..catch if CSS.escape available

Co-authored-by: Bryan Murphy <bryan87@gmail.com>
Co-authored-by: Stefan Cameron <stefan@stefcameron.com>
  • Loading branch information
@tidychips @stefcameron
3 people committed Dec 16, 2020
1 parent 485a0f9 commit c048203
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/soft-days-invite.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'tabbable': patch
---

fix crash when radio button name attributes contain CSS selector special characters (#168)
2 changes: 2 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Basically IE9+.

Why? It uses [Element.querySelectorAll()](https://developer.mozilla.org/en-US/docs/Web/API/Element/querySelectorAll) and [Window.getComputedStyle()](https://developer.mozilla.org/en-US/docs/Web/API/Window/getComputedStyle).

**Note:** When used with any version of IE, [CSS.escape](https://developer.mozilla.org/en-US/docs/Web/API/CSS/escape) needs a [polyfill](https://www.npmjs.com/package/css.escape) for tabbable to work properly with radio buttons that have `name` attributes containing special characters.

## Installation

```
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"rollup": "^2.34.2",
"rollup-plugin-sourcemaps": "^0.6.3",
"rollup-plugin-terser": "^7.0.2",
"sinon": "^9.2.2",
"typescript": "^4.1.2",
"watchify": "^3.11.1"
}
Expand Down
30 changes: 27 additions & 3 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,33 @@ const isTabbableRadio = function (node) {
return true;
}
const radioScope = node.form || node.ownerDocument;
const radioSet = radioScope.querySelectorAll(
'input[type="radio"][name="' + node.name + '"]'
);

const queryRadios = function (name) {
return radioScope.querySelectorAll(
'input[type="radio"][name="' + name + '"]'
);
};

let radioSet;
if (
typeof window !== 'undefined' &&
typeof window.CSS !== 'undefined' &&
typeof window.CSS.escape === 'function'
) {
radioSet = queryRadios(window.CSS.escape(node.name));
} else {
try {
radioSet = queryRadios(node.name);
} catch (err) {
// eslint-disable-next-line no-console
console.error(
'Looks like you have a radio button with a name attribute containing invalid CSS selector characters and need the CSS.escape polyfill: %s',
err.message
);
return false;
}
}

const checked = getCheckedRadio(radioSet, node.form);
return !checked || checked === node;
};
Expand Down
10 changes: 8 additions & 2 deletions test/fixtures/radio.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,19 @@
type="radio"
name="groupB"
value="a"
id="noform-other-group-radioA"
id="noform-groupB-radioA"
/>
<input
type="radio"
name="groupB"
value="b"
id="noform-other-group-radioB"
id="noform-groupB-radioB"
/>
</fieldset>

<fieldset>
<legend>no form groupC - names with CSS selector characters</legend>
<input type="radio" name="&quot;groupC&quot;" value="a" checked id="noform-groupC-radioA" />
<input type="radio" name="&quot;groupC&quot;" value="b" id="noform-groupC-radioB" />
</fieldset>
</div>
39 changes: 35 additions & 4 deletions test/tabbable.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const assert = require('chai').assert;
const sinon = require('sinon');
const {
tabbable,
isTabbable,
Expand Down Expand Up @@ -237,12 +238,40 @@ describe('tabbable', () => {
'formB-radioA',
'formB-radioB',
'noform-radioA',
'noform-other-group-radioA',
'noform-other-group-radioB',
'noform-groupB-radioA',
'noform-groupB-radioB',
'noform-groupC-radioA',
];
assert.deepEqual(actual, expected);
});

it('radio without CSS.escape', () => {
const actualEscape = CSS.escape;
CSS.escape = undefined;
const consoleError = sinon.stub(console, 'error');

try {
const actual = assertionSet.getFixture('radio').getTabbableIds();
const expected = [
'formA-radioA',
'formB-radioA',
'formB-radioB',
'noform-radioA',
'noform-groupB-radioA',
'noform-groupB-radioB',
];
assert.deepEqual(actual, expected);
// console.error should be called once per radio button with a name
// containing invalid CSS selector characters.
sinon.assert.calledTwice(consoleError);
} finally {
if (actualEscape) {
CSS.escape = actualEscape;
}
consoleError.restore();
}
});

it('details', () => {
const actual = assertionSet.getFixture('details').getTabbableIds();
const expected = [
Expand Down Expand Up @@ -411,8 +440,10 @@ describe('tabbable', () => {
'formB-radioB',
'noform-radioA',
'noform-radioB',
'noform-other-group-radioA',
'noform-other-group-radioB',
'noform-groupB-radioA',
'noform-groupB-radioB',
'noform-groupC-radioA',
'noform-groupC-radioB',
];
assert.deepEqual(actual.sort(), expected.sort());
});
Expand Down
81 changes: 79 additions & 2 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,42 @@
estree-walker "^1.0.1"
picomatch "^2.2.2"

"@sinonjs/commons@^1", "@sinonjs/commons@^1.6.0", "@sinonjs/commons@^1.7.0", "@sinonjs/commons@^1.8.1":
version "1.8.1"
resolved "https://registry.yarnpkg.com/@sinonjs/commons/-/commons-1.8.1.tgz#e7df00f98a203324f6dc7cc606cad9d4a8ab2217"
integrity sha512-892K+kWUUi3cl+LlqEWIDrhvLgdL79tECi8JZUyq6IviKy/DNhuzCRlbHUjxK89f4ypPMMaFnFuR9Ie6DoIMsw==
dependencies:
type-detect "4.0.8"

"@sinonjs/fake-timers@^6.0.0", "@sinonjs/fake-timers@^6.0.1":
version "6.0.1"
resolved "https://registry.yarnpkg.com/@sinonjs/fake-timers/-/fake-timers-6.0.1.tgz#293674fccb3262ac782c7aadfdeca86b10c75c40"
integrity sha512-MZPUxrmFubI36XS1DI3qmI0YdN1gks62JtFZvxR67ljjSNCeK6U08Zx4msEWOXuofgqUt6zPHSi1H9fbjR/NRA==
dependencies:
"@sinonjs/commons" "^1.7.0"

"@sinonjs/formatio@^5.0.1":
version "5.0.1"
resolved "https://registry.yarnpkg.com/@sinonjs/formatio/-/formatio-5.0.1.tgz#f13e713cb3313b1ab965901b01b0828ea6b77089"
integrity sha512-KaiQ5pBf1MpS09MuA0kp6KBQt2JUOQycqVG1NZXvzeaXe5LGFqAKueIS0bw4w0P9r7KuBSVdUk5QjXsUdu2CxQ==
dependencies:
"@sinonjs/commons" "^1"
"@sinonjs/samsam" "^5.0.2"

"@sinonjs/samsam@^5.0.2", "@sinonjs/samsam@^5.3.0":
version "5.3.0"
resolved "https://registry.yarnpkg.com/@sinonjs/samsam/-/samsam-5.3.0.tgz#1d2f0743dc54bf13fe9d508baefacdffa25d4329"
integrity sha512-hXpcfx3aq+ETVBwPlRFICld5EnrkexXuXDwqUNhDdr5L8VjvMeSRwyOa0qL7XFmR+jVWR4rUZtnxlG7RX72sBg==
dependencies:
"@sinonjs/commons" "^1.6.0"
lodash.get "^4.4.2"
type-detect "^4.0.8"

"@sinonjs/text-encoding@^0.7.1":
version "0.7.1"
resolved "https://registry.yarnpkg.com/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz#8da5c6530915653f3a1f38fd5f101d8c3f8079c5"
integrity sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==

"@types/color-name@^1.1.1":
version "1.1.1"
resolved "https://registry.yarnpkg.com/@types/color-name/-/color-name-1.1.1.tgz#1c1261bbeaa10a8055bbc5d8ab84b7b2afc846a0"
Expand Down Expand Up @@ -2551,7 +2587,7 @@ didyoumean@^1.2.1:
resolved "https://registry.yarnpkg.com/didyoumean/-/didyoumean-1.2.1.tgz#e92edfdada6537d484d73c0172fd1eba0c4976ff"
integrity sha1-6S7f2tplN9SE1zwBcv0eugxJdv8=

diff@4.0.2:
diff@4.0.2, diff@^4.0.2:
version "4.0.2"
resolved "https://registry.yarnpkg.com/diff/-/diff-4.0.2.tgz#60f3aecb89d5fae520c11aa19efc2bb982aade7d"
integrity sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==
Expand Down Expand Up @@ -4147,6 +4183,11 @@ jsonparse@^1.2.0:
resolved "https://registry.yarnpkg.com/jsonparse/-/jsonparse-1.3.1.tgz#3f4dae4a91fac315f71062f8521cc239f1366280"
integrity sha1-P02uSpH6wxX3EGL4UhzCOfE2YoA=

just-extend@^4.0.2:
version "4.1.1"
resolved "https://registry.yarnpkg.com/just-extend/-/just-extend-4.1.1.tgz#158f1fdb01f128c411dc8b286a7b4837b3545282"
integrity sha512-aWgeGFW67BP3e5181Ep1Fv2v8z//iBJfrvyTnq8wG86vEESwmonn1zPBJ0VfmT9CJq2FIT0VsETtrNFm2a+SHA==

karma-browserify@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/karma-browserify/-/karma-browserify-7.0.0.tgz#60f443f991ecfbbfb5737a9fd76e73b8b6eb9a57"
Expand Down Expand Up @@ -4322,6 +4363,11 @@ locate-path@^6.0.0:
dependencies:
p-locate "^5.0.0"

lodash.get@^4.4.2:
version "4.4.2"
resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99"
integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=

lodash.memoize@~3.0.3:
version "3.0.4"
resolved "https://registry.yarnpkg.com/lodash.memoize/-/lodash.memoize-3.0.4.tgz#2dcbd2c287cbc0a55cc42328bd0c736150d53e3f"
Expand Down Expand Up @@ -4705,6 +4751,17 @@ nice-try@^1.0.4:
resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366"
integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==

nise@^4.0.4:
version "4.0.4"
resolved "https://registry.yarnpkg.com/nise/-/nise-4.0.4.tgz#d73dea3e5731e6561992b8f570be9e363c4512dd"
integrity sha512-bTTRUNlemx6deJa+ZyoCUTRvH3liK5+N6VQZ4NIw90AgDXY6iPnsqplNFf6STcj+ePk0H/xqxnP75Lr0J0Fq3A==
dependencies:
"@sinonjs/commons" "^1.7.0"
"@sinonjs/fake-timers" "^6.0.0"
"@sinonjs/text-encoding" "^0.7.1"
just-extend "^4.0.2"
path-to-regexp "^1.7.0"

node-fetch@^2.6.0:
version "2.6.1"
resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052"
Expand Down Expand Up @@ -5109,6 +5166,13 @@ path-platform@~0.11.15:
resolved "https://registry.yarnpkg.com/path-platform/-/path-platform-0.11.15.tgz#e864217f74c36850f0852b78dc7bf7d4a5721bf2"
integrity sha1-6GQhf3TDaFDwhSt43Hv31KVyG/I=

path-to-regexp@^1.7.0:
version "1.8.0"
resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-1.8.0.tgz#887b3ba9d84393e87a0a0b9f4cb756198b53548a"
integrity sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==
dependencies:
isarray "0.0.1"

path-type@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/path-type/-/path-type-2.0.0.tgz#f012ccb8415b7096fc2daa1054c3d72389594c73"
Expand Down Expand Up @@ -5869,6 +5933,19 @@ simple-html-index@^1.4.0:
dependencies:
from2-string "^1.1.0"

sinon@^9.2.2:
version "9.2.2"
resolved "https://registry.yarnpkg.com/sinon/-/sinon-9.2.2.tgz#b83cf5d43838f99cfa3644453f4c7db23e7bd535"
integrity sha512-9Owi+RisvCZpB0bdOVFfL314I6I4YoRlz6Isi4+fr8q8YQsDPoCe5UnmNtKHRThX3negz2bXHWIuiPa42vM8EQ==
dependencies:
"@sinonjs/commons" "^1.8.1"
"@sinonjs/fake-timers" "^6.0.1"
"@sinonjs/formatio" "^5.0.1"
"@sinonjs/samsam" "^5.3.0"
diff "^4.0.2"
nise "^4.0.4"
supports-color "^7.1.0"

slash@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/slash/-/slash-3.0.0.tgz#6539be870c165adbd5240220dbe361f1bc4d4634"
Expand Down Expand Up @@ -6526,7 +6603,7 @@ type-check@~0.3.2:
dependencies:
prelude-ls "~1.1.2"

type-detect@^4.0.0, type-detect@^4.0.5:
type-detect@4.0.8, type-detect@^4.0.0, type-detect@^4.0.5, type-detect@^4.0.8:
version "4.0.8"
resolved "https://registry.yarnpkg.com/type-detect/-/type-detect-4.0.8.tgz#7646fb5f18871cfbb7749e69bd39a6388eb7450c"
integrity sha512-0fr/mIH1dlO+x7TlcMy+bIDqKPsw/70tVyeHW787goQjhmqaZe10uwLujubK9q9Lg6Fiho1KUKDYz0Z7k7g5/g==
Expand Down

0 comments on commit c048203

Please sign in to comment.