search

alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.18k stars 323 forks source link

Character count fails if ID contains characters that have a special meaning in CSS #1809

Closed 36degrees closed 3 years ago

36degrees commented 4 years ago

Because we interpolate the textarea ID into a querySelector call without escaping it, the character count can fail if the ID contains characters that have a special meaning in CSS:

https://github.com/alphagov/govuk-frontend/blob/387c33d5280745523a9ee0fd1da961be584137e6/src/govuk/components/character-count/character-count.js#L8-L10

<div class="govuk-character-count" data-module="govuk-character-count" data-maxlength="10">
  <div class="govuk-form-group">
  <label class="govuk-label" for="docs[more-detail]">
    Can you provide more detail?
  </label>
  <textarea class="govuk-textarea govuk-js-character-count" id="docs[more-detail]" name="more-detail" rows="5" aria-describedby="docs[more-detail]-info"></textarea>
</div>

  <span id="docs[more-detail]-info" class="govuk-hint govuk-character-count__message" aria-live="polite">
  You can enter up to 10 characters
</span>

</div>
Uncaught DOMException: Failed to execute 'querySelector' on 'Element': '[id=docs[more-detail]-info]' is not a valid selector.
    at new CharacterCount (http://localhost:3000/public/all.js:1501:34)
    at http://localhost:3000/public/all.js:2366:5
    at NodeList.forEach (<anonymous>)
    at nodeListForEach (http://localhost:3000/public/all.js:14:18)
    at Object.initAll (http://localhost:3000/public/all.js:2365:3)
    at http://localhost:3000/components/character-count/special-chars/preview:69:32
36degrees commented 4 years ago

This is similar to #1808, and the solution is probably the same – wrapping the call with CSS.escape – although we should be able to use a simpler ID selector rather than using the attribute syntax:

this.$countMessage = $module.querySelector('#' + CSS.escape(this.$textarea.id) + '-info')
36degrees commented 4 years ago

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.

36degrees commented 3 years ago

I think this might actually have been fixed by quoting the ID attribute which was done in #2080, except possibly if the ID itself contains double quotes?