Conditional reveals fail if ID contains characters that have a special meaning in CSS

[36degrees]

When radios and checkboxes with conditional reveals are initialised, we check that the element targeted by aria-controls exists:

govuk-frontend/src/govuk/components/radios/radios.js

Lines 20 to 26 in 387c33d

	var controls = $input.getAttribute('data-aria-controls')
	// Check if input controls anything
	// Check if content exists, before setting attributes.
	if (!controls || !$module.querySelector('#' + controls)) {
	return
	}

If the conditionally revealed element has an ID that contains characters that have a special meaning in CSS, such as [], this fails because the ID needs to be escaped:

<div class="govuk-form-group">
<fieldset class="govuk-fieldset">
  <legend class="govuk-fieldset__legend">
    How do you want to be contacted?
  </legend>
  <div class="govuk-radios govuk-radios--conditional" data-module="govuk-radios">
        <div class="govuk-radios__item">
          <input class="govuk-radios__input" id="person-1[how-contacted]" name="person-1[how-contacted]" type="radio" value="email" data-aria-controls="conditional-person-1[how-contacted]">
          <label class="govuk-label govuk-radios__label" for="person-1[how-contacted]">
        Email
      </label>
        </div>
          <div class="govuk-radios__conditional govuk-radios__conditional--hidden" id="conditional-person-1[how-contacted]">
            <label class="govuk-label" for="context-email">Mobile phone number</label>
<input class="govuk-input govuk-!-width-one-third" name="context-email" type="text" id="context-email">

          </div>
        <div class="govuk-radios__item">
          <input class="govuk-radios__input" id="person-1[how-contacted]-2" name="person-1[how-contacted]" type="radio" value="phone" data-aria-controls="conditional-person-1[how-contacted]-2">
          <label class="govuk-label govuk-radios__label" for="person-1[how-contacted]-2">
        Phone
      </label>
        </div>
          <div class="govuk-radios__conditional govuk-radios__conditional--hidden" id="conditional-person-1[how-contacted]-2">
            <label class="govuk-label" for="contact-phone">Phone number</label>
<input class="govuk-input govuk-!-width-one-third" name="contact-phone" type="text" id="contact-phone">

          </div>
        <div class="govuk-radios__item">
          <input class="govuk-radios__input" id="person-1[how-contacted]-3" name="person-1[how-contacted]" type="radio" value="text" data-aria-controls="conditional-person-1[how-contacted]-3">
          <label class="govuk-label govuk-radios__label" for="person-1[how-contacted]-3">
        Text message
      </label>
        </div>
          <div class="govuk-radios__conditional govuk-radios__conditional--hidden" id="conditional-person-1[how-contacted]-3">
            <label class="govuk-label" for="contact-text-message">Mobile phone number</label>
<input class="govuk-input govuk-!-width-one-third" name="contact-text-message" type="text" id="contact-text-message">

          </div>
  </div>
</fieldset>
</div>

Uncaught DOMException: Failed to execute 'querySelector' on 'Element': '#conditional-person-1[how-contacted]-2' is not a valid selector.
    at Radios.<anonymous> (http://localhost:3000/public/all.js:1978:31)
    at NodeList.forEach (<anonymous>)
    at nodeListForEach (http://localhost:3000/public/all.js:14:18)
    at Radios.init (http://localhost:3000/public/all.js:1973:3)
    at http://localhost:3000/public/all.js:2384:24
    at NodeList.forEach (<anonymous>)
    at nodeListForEach (http://localhost:3000/public/all.js:14:18)
    at Object.initAll (http://localhost:3000/public/all.js:2383:3)
    at http://localhost:3000/components/radios/with-conditional-items-and-square-brackets/preview:98:32

In some cases, the conditional reveal may fail silently, for example if the generated ID makes a valid CSS selector:

<div class="govuk-form-group">
	<fieldset class="govuk-fieldset">
		<legend class="govuk-fieldset__legend govuk-fieldset__legend--m">
			<h1 class="govuk-fieldset__heading">Was it due to candidate behaviour?</h1>
		</legend>
		<div class="govuk-radios govuk-radios--conditional" data-module="govuk-radios">
			<div class="govuk-radios__item">
				<input class="govuk-radios__input" id="rejectionReasons[candidate-actions]" name="rejectionReasons[candidate-actions]" type="radio" value="Yes" data-aria-controls="conditional-rejectionReasons[candidate-actions]">
				<label class="govuk-label govuk-radios__label" for="rejectionReasons[candidate-actions]">
                    Yes
                </label>
			</div>
			<div class="govuk-radios__conditional govuk-radios__conditional--hidden" id="conditional-rejectionReasons[candidate-actions]">
                Blah
            </div>
			<div class="govuk-radios__item">
				<input class="govuk-radios__input" id="rejectionReasons[candidate-actions]-2" name="rejectionReasons[candidate-actions]" type="radio" value="No">
				<label class="govuk-label govuk-radios__label" for="rejectionReasons[candidate-actions]-2">
                    No
                </label>
			</div>
		</div>
	</fieldset>
</div>

In this case, the evaluated selector would be:

conditional-rejectionReasons[candidate-actions]

Which is interpreted as element with ID conditional-rejectionReasons, also having attribute candidate-actions, which does not match any element on the page, and so the initialise function returns early.

If it was correctly escaped, it should be:

conditional-rejectionReasons\[candidate-actions\]

This is especially problematic where we automatically generate IDs based on name, as it's more likely that users will use a name that includes [].

[36degrees]

One solution to this may be to use CSS.escape, although we'd need to include a polyfill for it:

if (!controls || !$module.querySelector('#' + CSS.escape(controls))) {

[36degrees]

There's a potential fix (using document.getElementById) in #1843, but this might cause issues for anyone who might be using duplicate IDs – which whilst invalid HTML would work 'correctly' (except for the aria-controls association) with the current approach.

Attaching this to the 4.0 milestone so we can flag it as a potential breaking change in the release notes.

[frankieroberto]

@36degrees I hit this bug today. My hunch was that switching to getElementById would be the simplest/best fix.

[36degrees]

Based on #1843 fixing this, changing the label from 'Days' to 'Hours' as I think we have a solution that works that doesn't involve polyfilling CSS.escape.