Ensure <li>s are wrapped in a <ol> or <ul> after sanitizing HTML

src/amo/purify.js

@@ -2,4 +2,30 @@ import createDOMPurify from 'dompurify';
 
 import universalWindow from 'amo/window';
 
-export default createDOMPurify(universalWindow);
+const _purify = createDOMPurify(universalWindow);
+_purify.addHook('uponSanitizeElement', (node, data, config) => {
+  const _ALLOWED_TAGS = config.ALLOWED_TAGS || [];
+  if (
+    node.tagName &&
+    node.parentNode &&
+    node.tagName === 'LI' &&
+    !['MENU', 'OL', 'UL'].includes(node.parentNode.tagName) &&
+    _ALLOWED_TAGS.includes('ul') &&
+    _ALLOWED_TAGS.includes('li')
+  ) {
+    // If we find a <li> with no <ul>/<ol>/<menu> parent, create one for it.
+    // Try to use the same <ul> if there are multiple <li> next to each other,
+    // otherwise create a new <ul>, it's better than creating invalid HTML that
+    // would throw the virtual DOM off anyway.
+    const oldParent = node.parentNode;
+    const previousElement = node.previousElementSibling;
+    const newParent =
+      previousElement && previousElement.tagName === 'UL'
+        ? previousElement
+        : node.ownerDocument.createElement('ul');
+    oldParent.insertBefore(newParent, node);
+    newParent.appendChild(node);
+  }
+});
+
+export default _purify;

tests/unit/amo/utils/test_index.js

@@ -698,6 +698,97 @@ describe(__filename, () => {
         __html: '<a href="http://example.org">link</a>',
       });
     });
+
+    describe('custom `<li>` handling through hook', () => {
+      it('removes `<li>` if not allowed', () => {
+        const html = '<li>witness me!</li>';
+        expect(sanitizeHTML(html, [])).toEqual({
+          __html: 'witness me!',
+        });
+
+        expect(sanitizeHTML(html)).toEqual({
+          __html: 'witness me!',
+        });
+      });
+
+      it('does not wrap `<li>` inside a `<ul>` if not allowed', () => {
+        const html = '<li>witness me!</li>';
+        expect(sanitizeHTML(html, ['li'])).toEqual({
+          __html: '<li>witness me!</li>',
+        });
+      });
+
+      it('wraps `<li>` inside a `<ul>` if they dont already', () => {
+        const html = '<li>witness me!</li>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>witness me!</li></ul>',
+        });
+      });
+
+      it('wraps `<li>` inside a `<ul>` if their parent was not allowed', () => {
+        const html = '<ol><li>witness me!</li></ol>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>witness me!</li></ul>',
+        });
+      });
+
+      it('doesnt wrap `<li>` inside a `<ul>` if not necessary', () => {
+        const html = '<ul><li>witness me!</li></ul>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>witness me!</li></ul>',
+        });
+      });
+
+      it('doesnt wrap `<li>` inside a `<ul>` if not necessary with ol', () => {
+        const html = '<ol><li>witness me!</li></ol>';
+        expect(sanitizeHTML(html, ['li', 'ol', 'ul'])).toEqual({
+          __html: '<ol><li>witness me!</li></ol>',
+        });
+      });
+
+      it('doesnt wrap `<li>` inside a `<ul>` if not necessary with menu', () => {
+        const html = '<menu><li>witness me!</li></menu>';
+        expect(sanitizeHTML(html, ['li', 'menu', 'ul'])).toEqual({
+          __html: '<menu><li>witness me!</li></menu>',
+        });
+      });
+
+      it('handles multiple `<li>`', () => {
+        const html = '<li>foo</li><li>bar</li>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>foo</li><li>bar</li></ul>',
+        });
+      });
+
+      it('handles multiple `<li>` when one does not need fixing', () => {
+        const html = '<li>foo</li><ul><li>bar</li></ul>';
+        expect(sanitizeHTML(html, ['li', 'ul'])).toEqual({
+          __html: '<ul><li>foo</li></ul><ul><li>bar</li></ul>',
+        });
+      });
+
+      it('handles multiple `<li>` in separate lists', () => {
+        const html = '<li>foo</li><code>alice</code><li>bar</li>';
+        expect(sanitizeHTML(html, ['li', 'ul', 'code'])).toEqual({
+          __html:
+            '<ul><li>foo</li></ul><code>alice</code><ul><li>bar</li></ul>',
+        });
+      });
+
+      it('handles no closing `</li>`', () => {
+        const html = '<li>foo<li>bar';
+        expect(sanitizeHTML(html, ['li', 'ul', 'p'])).toEqual({
+          __html: '<ul><li>foo</li><li>bar</li></ul>',
+        });
+      });
+
+      it('does not mess ordering`', () => {
+        const html = 'Before <li>foo</li><li>bar</li> After';
+        expect(sanitizeHTML(html, ['li', 'ul', 'p'])).toEqual({
+          __html: 'Before <ul><li>foo</li><li>bar</li></ul> After',
+        });
+      });
+    });
   });
 
   describe('removeProtocolFromURL', () => {