Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #2274
36degrees opened this issue May 15, 2020 · 3 comments · Fixed by #2408
Closed
Tracked by #2274
Assignees
Labels
character count 🕔 hours A well understood issue which we expect to take less than a day to resolve. javascript
Milestone

Comments

@36degrees
Copy link
Contributor

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:

if (this.$textarea) {
this.$countMessage = $module.querySelector('[id=' + this.$textarea.id + '-info]')
}

<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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 36degrees added 🕔 hours A well understood issue which we expect to take less than a day to resolve. and removed 🕔 days labels Aug 24, 2020
@36degrees
Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
character count 🕔 hours A well understood issue which we expect to take less than a day to resolve. javascript
Projects
2 participants