Skip to content

Commit

Permalink
Use document.getElementById to check target exists
Browse files Browse the repository at this point in the history
Because the string passed to `querySelector` is evaluated as a CSS selector, this can fail if the ID contains characters that have a special meaning in CSS, such as `.` or `[]` – the ID would need to be escaped for it to be evaluated correctly.

Avoid this by using `document.getElementById` instead, so the ID is no longer evaluated as a selector.

The downside is that we can't constrain the scope to the $module. However, we don't think this should cause any issues as the ID _should_ be unique within the document.

This fixes the failing tests introduced in the previous commit.
  • Loading branch information
36degrees committed Oct 14, 2021
1 parent 5594370 commit 266f877
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/govuk/components/checkboxes/checkboxes.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Checkboxes.prototype.init = function () {

// Skip checkboxes without data-aria-controls attributes, or where the
// target element does not exist.
if (!target || !$module.querySelector('#' + target)) {
if (!target || !document.getElementById(target)) {
return
}

Expand Down
2 changes: 1 addition & 1 deletion src/govuk/components/radios/radios.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Radios.prototype.init = function () {

// Skip radios without data-aria-controls attributes, or where the
// target element does not exist.
if (!target || !$module.querySelector('#' + target)) {
if (!target || !document.getElementById(target)) {
return
}

Expand Down

0 comments on commit 266f877

Please sign in to comment.