From 8251072e6d4ac69f1add12e2da1b454cec573647 Mon Sep 17 00:00:00 2001 From: LB Johnston Date: Sat, 10 Jun 2023 11:19:10 +1000 Subject: [PATCH] Replace usage of innerHTML with textContent - Stimulus CountController content is programatically generated but best to avoid writing HTML accidentally - ChooserModel field required label should avoid risk of translations with HTML - Image focal point chooser's label does not need to support HTML - FieldBlock us using h util but this can be avoided by built in browser escaping when innerText is used - focal-point-chooser gets value from its set up but we should avoid innerHTML if we can --- CHANGELOG.txt | 1 + client/src/components/StreamField/blocks/FieldBlock.js | 8 +++++--- client/src/controllers/CountController.ts | 4 ++-- client/src/controllers/UpgradeController.test.js | 2 +- client/src/controllers/UpgradeController.ts | 6 ++---- client/src/includes/chooserModal.js | 2 +- docs/releases/5.1.md | 1 + .../static_src/wagtailimages/js/focal-point-chooser.js | 2 +- 8 files changed, 14 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 7df58c0919..d3281db686 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -34,6 +34,7 @@ Changelog * Fix: Fix the lock description message missing the model_name variable when locked only by system (Sébastien Corbin) * Fix: Fix empty blocks created in migration operations (Sandil Ranasinghe) * Fix: Ensure that gettext_lazy works correctly when using verbose_name on a generic Settings models (Sébastien Corbin) + * Fix: Remove unnecessary usage of `innerHTML` when modifying DOM content (LB (Ben) Johnston) * Docs: Document how to add non-ModelAdmin views to a `ModelAdminGroup` (Onno Timmerman) * Docs: Document how to add StructBlock data to a StreamField (Ramon Wenger) * Docs: Update ReadTheDocs settings to v2 to resolve urllib3 issue in linkcheck extension (Thibaud Colas) diff --git a/client/src/components/StreamField/blocks/FieldBlock.js b/client/src/components/StreamField/blocks/FieldBlock.js index 5102740a88..7262c22d95 100644 --- a/client/src/components/StreamField/blocks/FieldBlock.js +++ b/client/src/components/StreamField/blocks/FieldBlock.js @@ -125,9 +125,11 @@ export class FieldBlock { const errorElement = document.createElement('p'); errorElement.classList.add('error-message'); - errorElement.innerHTML = error.messages - .map((message) => `${h(message)}`) - .join(''); + error.messages.forEach((message) => { + const messageItem = document.createElement('span'); + messageItem.textContent = message; + errorElement.appendChild(messageItem); + }); errorContainer.appendChild(errorElement); } else { this.field.classList.remove('w-field--error'); diff --git a/client/src/controllers/CountController.ts b/client/src/controllers/CountController.ts index 975367e5d5..fef4625bba 100644 --- a/client/src/controllers/CountController.ts +++ b/client/src/controllers/CountController.ts @@ -79,10 +79,10 @@ export class CountController extends Controller { this.element.classList.toggle(this.activeClass, total > min); } if (this.hasLabelTarget) { - this.labelTarget.innerHTML = total > min ? this.getLabel(total) : ''; + this.labelTarget.textContent = total > min ? this.getLabel(total) : ''; } if (this.hasTotalTarget) { - this.totalTarget.innerHTML = total > min ? `${total}` : ''; + this.totalTarget.textContent = total > min ? `${total}` : ''; } } } diff --git a/client/src/controllers/UpgradeController.test.js b/client/src/controllers/UpgradeController.test.js index 05558ad878..0beeeb8409 100644 --- a/client/src/controllers/UpgradeController.test.js +++ b/client/src/controllers/UpgradeController.test.js @@ -74,7 +74,7 @@ describe('UpgradeController', () => { ).toBe(false); // should update the latest version number in the text - expect(document.getElementById('latest-version').innerText).toBe( + expect(document.getElementById('latest-version').textContent).toBe( data.version, ); diff --git a/client/src/controllers/UpgradeController.ts b/client/src/controllers/UpgradeController.ts index 93fcd9692f..f2c603748e 100644 --- a/client/src/controllers/UpgradeController.ts +++ b/client/src/controllers/UpgradeController.ts @@ -78,12 +78,10 @@ export class UpgradeController extends Controller { } if (this.latestVersionTarget instanceof HTMLElement) { - this.latestVersionTarget.innerText = [ - data.version, - showLTSOnly ? '(LTS)' : '', - ] + const versionLabel = [data.version, showLTSOnly ? '(LTS)' : ''] .join(' ') .trim(); + this.latestVersionTarget.textContent = versionLabel; } if (this.linkTarget instanceof HTMLElement) { diff --git a/client/src/includes/chooserModal.js b/client/src/includes/chooserModal.js index 13b187e38c..59fcfc30b3 100644 --- a/client/src/includes/chooserModal.js +++ b/client/src/includes/chooserModal.js @@ -19,7 +19,7 @@ const validateCreationForm = (form) => { } const errorElement = document.createElement('p'); errorElement.classList.add('error-message'); - errorElement.innerHTML = gettext('This field is required.'); + errorElement.textContent = gettext('This field is required.'); errors.appendChild(errorElement); } } diff --git a/docs/releases/5.1.md b/docs/releases/5.1.md index 019fc77fab..2787c89cff 100644 --- a/docs/releases/5.1.md +++ b/docs/releases/5.1.md @@ -65,6 +65,7 @@ Thank you to Damilola for his work, and to Google for sponsoring this project. * Fix the lock description message missing the model_name variable when locked only by system (Sébastien Corbin) * Fix empty blocks created in migration operations (Sandil Ranasinghe) * Ensure that `gettext_lazy` works correctly when using `verbose_name` on a generic Settings models (Sébastien Corbin) + * Remove unnecessary usage of `innerHTML` when modifying DOM content (LB (Ben) Johnston) ### Documentation diff --git a/wagtail/images/static_src/wagtailimages/js/focal-point-chooser.js b/wagtail/images/static_src/wagtailimages/js/focal-point-chooser.js index 26ffa3487a..c2380de406 100644 --- a/wagtail/images/static_src/wagtailimages/js/focal-point-chooser.js +++ b/wagtail/images/static_src/wagtailimages/js/focal-point-chooser.js @@ -45,7 +45,7 @@ function setupJcrop(image, original, focalPointOriginal, fields) { const label = document.createElement('label'); label.setAttribute('for', id); label.classList.add('visuallyhidden'); - label.innerHTML = labelContent; + label.textContent = labelContent; var holder = document.getElementsByClassName('jcrop-holder'); holder[0].prepend(label); },