Skip to content

Commit cf4ca54

Browse files
ErykKulErykKul
committed
Parse featured item HTML with jsdom instead of regex, and compare against the content sent
The tokenizer split tags on the first '>', so a quoted attribute value such as title="A > B" was cut in two and its tag was never attribute-sorted. Parsing and serializing with jsdom, which jest-environment-jsdom already brings in, leaves only attribute sorting and block-boundary whitespace collapsing to do by hand. The functional test now compares the returned content against the content it sent rather than against a copy of the server's pretty-printed output, which is what the normalizer was meant to make unnecessary. That copy stays as SERVER_FORMATTED_CONTENT_FIELD_WITH_ALL_TAGS, used by the unit test to prove the normalizer absorbs real server formatting. The fixture gains a paragraph whose inline elements are separated only by a space, so the whitespace the normalizer refuses to collapse is exercised against a real Dataverse round trip.
1 parent 3348e1c commit cf4ca54

6 files changed

Lines changed: 100 additions & 90 deletions

File tree

‎package-lock.json‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"homepage": "https://github.com/IQSS/dataverse-client-javascript#readme",
4444
"devDependencies": {
4545
"@types/jest": "^29.5.12",
46+
"@types/jsdom": "^20.0.1",
4647
"@typescript-eslint/eslint-plugin": "5.51.0",
4748
"@typescript-eslint/parser": "5.51.0",
4849
"@web-std/file": "3.0.3",
@@ -56,6 +57,7 @@
5657
"husky": "9.1.7",
5758
"jest": "^29.4.3",
5859
"jest-environment-jsdom": "29.7.0",
60+
"jsdom": "^20.0.3",
5961
"prettier": "2.8.4",
6062
"testcontainers": "^10.11.0",
6163
"ts-jest": "^29.0.5",

‎test/functional/collections/UpdateCollectionFeaturedItems.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
CONTENT_FIELD_WITH_ALL_TAGS,
1919
createCollectionViaApi,
2020
deleteCollectionViaApi,
21-
EXPECTED_CONTENT_FIELD_WITH_ALL_TAGS,
2221
publishCollectionViaApi
2322
} from '../../testHelpers/collections/collectionHelper'
2423
import {
@@ -167,7 +166,7 @@ describe('execute', () => {
167166
expect(secondItemResponse.imageFileName).toBeUndefined()
168167

169168
expect(normalizeHtml(thirdItemResponse.content)).toEqual(
170-
normalizeHtml(EXPECTED_CONTENT_FIELD_WITH_ALL_TAGS)
169+
normalizeHtml(CONTENT_FIELD_WITH_ALL_TAGS)
171170
)
172171
expect(thirdItemResponse.displayOrder).toBe(newFeaturedItems[2].displayOrder)
173172
expect(thirdItemResponse.imageFileName).toEqual('featured-item-test-image-3.png')

‎test/testHelpers/collections/collectionHelper.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,12 @@ export const createCollectionFacetRequestPayload = (): CollectionFacetPayload =>
268268
}
269269

270270
export const CONTENT_FIELD_WITH_ALL_TAGS =
271-
'<h1 class="rte-heading">A title</h1><p class="rte-paragraph">Esto es una oracion que contiene texto en <strong class="rte-bold">negrita</strong>, <em class="rte-italic">italica</em>, <u class="rte-underline">subrayada</u>, <s class="rte-strike">tachado</s>, <code class="rte-code">de tipo code</code>, este es <a target="_blank" rel="noopener noreferrer nofollow" class="rte-link" href="https://youtube.com">un link que apunta a youtube</a>.</p><p class="rte-paragraph">Una lista desordenada:</p><ul class="rte-bullet-list"><li><p class="rte-paragraph">Item</p></li><li><p class="rte-paragraph">Item</p></li></ul><p class="rte-paragraph">Una lista ordenada:</p><ol class="rte-ordered-list"><li><p class="rte-paragraph">Item 1</p></li><li><p class="rte-paragraph">Item 2</p></li></ol><blockquote class="rte-blockquote"><p class="rte-paragraph">Este es un blockquote.</p></blockquote><p class="rte-paragraph">Esto que viene es un bloque de codigo.</p><pre class="rte-code-block"><code> &lt;Controller name={`featuredItems.${itemIndex}.content`} control={control} rules={rules} render={({ field: { onChange, ref, value }, fieldState: { invalid, error } }) =&gt; { console.log({ value }) return ( &lt;Col&gt; &lt;RichTextEditor initialValue={value as string} editorContentAriaLabelledBy={`featuredItems.${itemIndex}.content`} onChange={onChange} invalid={invalid} ariaRequired ref={ref} /&gt; {invalid &amp;&amp; &lt;div className={styles["error-msg"]}&gt;{error?.message}&lt;/div&gt;} &lt;/Col&gt; ) }} /&gt;</code></pre>'
271+
'<h1 class="rte-heading">A title</h1><p class="rte-paragraph">Esto es una oracion que contiene texto en <strong class="rte-bold">negrita</strong>, <em class="rte-italic">italica</em>, <u class="rte-underline">subrayada</u>, <s class="rte-strike">tachado</s>, <code class="rte-code">de tipo code</code>, este es <a target="_blank" rel="noopener noreferrer nofollow" class="rte-link" href="https://youtube.com">un link que apunta a youtube</a>.</p><p class="rte-paragraph"><strong class="rte-bold">Negrita</strong> <em class="rte-italic">seguida de italica</em></p><p class="rte-paragraph">Una lista desordenada:</p><ul class="rte-bullet-list"><li><p class="rte-paragraph">Item</p></li><li><p class="rte-paragraph">Item</p></li></ul><p class="rte-paragraph">Una lista ordenada:</p><ol class="rte-ordered-list"><li><p class="rte-paragraph">Item 1</p></li><li><p class="rte-paragraph">Item 2</p></li></ol><blockquote class="rte-blockquote"><p class="rte-paragraph">Este es un blockquote.</p></blockquote><p class="rte-paragraph">Esto que viene es un bloque de codigo.</p><pre class="rte-code-block"><code> &lt;Controller name={`featuredItems.${itemIndex}.content`} control={control} rules={rules} render={({ field: { onChange, ref, value }, fieldState: { invalid, error } }) =&gt; { console.log({ value }) return ( &lt;Col&gt; &lt;RichTextEditor initialValue={value as string} editorContentAriaLabelledBy={`featuredItems.${itemIndex}.content`} onChange={onChange} invalid={invalid} ariaRequired ref={ref} /&gt; {invalid &amp;&amp; &lt;div className={styles["error-msg"]}&gt;{error?.message}&lt;/div&gt;} &lt;/Col&gt; ) }} /&gt;</code></pre>'
272272

273-
export const EXPECTED_CONTENT_FIELD_WITH_ALL_TAGS =
273+
export const SERVER_FORMATTED_CONTENT_FIELD_WITH_ALL_TAGS =
274274
'<h1 class="rte-heading">A title</h1>\n' +
275275
'<p class="rte-paragraph">Esto es una oracion que contiene texto en <strong class="rte-bold">negrita</strong>, <em class="rte-italic">italica</em>, <u class="rte-underline">subrayada</u>, <s class="rte-strike">tachado</s>, <code class="rte-code">de tipo code</code>, este es <a target="_blank" rel="noopener noreferrer nofollow" class="rte-link" href="https://youtube.com">un link que apunta a youtube</a>.</p>\n' +
276+
'<p class="rte-paragraph"><strong class="rte-bold">Negrita</strong> <em class="rte-italic">seguida de italica</em></p>\n' +
276277
'<p class="rte-paragraph">Una lista desordenada:</p>\n' +
277278
'<ul class="rte-bullet-list">\n' +
278279
' <li><p class="rte-paragraph">Item</p></li>\n' +

‎test/testHelpers/html/htmlNormalizer.ts‎

Lines changed: 55 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { JSDOM } from 'jsdom'
2+
13
const WHITESPACE_SENSITIVE_TAGS = new Set(['pre', 'textarea'])
24

35
const BLOCK_TAGS = new Set([
@@ -43,103 +45,73 @@ const BLOCK_TAGS = new Set([
4345
'ul'
4446
])
4547

46-
const TAG_PATTERN = /^<\s*(\/?)\s*([a-zA-Z][\w:-]*)([\s\S]*?)(\/?)\s*>$/
47-
const ATTRIBUTE_PATTERN = /([\w:-]+)(?:\s*=\s*("[^"]*"|'[^']*'|[^\s"'>]+))?/g
48+
const document = new JSDOM('').window.document
4849

49-
interface ParsedTag {
50-
closing: boolean
51-
name: string
52-
selfClosing: boolean
53-
}
50+
const isElement = (node: Node | null | undefined): node is Element =>
51+
node !== null && node !== undefined && node.nodeType === node.ELEMENT_NODE
5452

55-
const parseTag = (token: string): ParsedTag | undefined => {
56-
const match = TAG_PATTERN.exec(token)
57-
if (match === null) {
58-
return undefined
59-
}
60-
return {
61-
closing: match[1] === '/',
62-
name: match[2].toLowerCase(),
63-
selfClosing: match[4] === '/'
64-
}
65-
}
53+
const isText = (node: Node): node is Text => node.nodeType === node.TEXT_NODE
6654

67-
const normalizeTag = (token: string): string => {
68-
const match = TAG_PATTERN.exec(token)
69-
if (match === null) {
70-
return token
71-
}
72-
const [, closing, name, attributeSource, selfClosing] = match
73-
const attributes = Array.from(attributeSource.matchAll(ATTRIBUTE_PATTERN))
74-
.map(([, attributeName, attributeValue]) =>
75-
attributeValue === undefined
76-
? attributeName.toLowerCase()
77-
: `${attributeName.toLowerCase()}=${normalizeAttributeValue(attributeValue)}`
78-
)
79-
.sort()
80-
const renderedAttributes = attributes.length === 0 ? '' : ` ${attributes.join(' ')}`
81-
return `<${closing}${name.toLowerCase()}${renderedAttributes}${selfClosing}>`
82-
}
55+
const tagNameOf = (element: Element): string => element.tagName.toLowerCase()
8356

84-
const normalizeAttributeValue = (value: string): string => {
85-
const unquoted =
86-
(value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))
87-
? value.slice(1, -1)
88-
: value
89-
return `"${unquoted}"`
90-
}
57+
const isBlockBoundary = (sibling: Node | undefined, parent: Node): boolean =>
58+
sibling === undefined
59+
? !isElement(parent) || BLOCK_TAGS.has(tagNameOf(parent))
60+
: isElement(sibling) && BLOCK_TAGS.has(tagNameOf(sibling))
9161

92-
const isBlockBoundary = (token: string | undefined): boolean => {
93-
if (token === undefined) {
94-
return true
95-
}
96-
const tag = parseTag(token)
97-
return tag !== undefined && BLOCK_TAGS.has(tag.name)
62+
const sortAttributes = (element: Element): void => {
63+
const attributes = Array.from(element.attributes).sort((one, other) =>
64+
one.name.localeCompare(other.name)
65+
)
66+
attributes.forEach((attribute) => element.removeAttribute(attribute.name))
67+
attributes.forEach((attribute) => element.setAttribute(attribute.name, attribute.value))
9868
}
9969

100-
export const normalizeHtml = (html: string): string => {
101-
const tokens = html.split(/(<[^>]*>)/).filter((token) => token !== '')
102-
const normalized: string[] = []
103-
let whitespaceSensitiveDepth = 0
104-
105-
tokens.forEach((token, index) => {
106-
const tag = token.startsWith('<') ? parseTag(token) : undefined
107-
108-
if (tag !== undefined) {
109-
if (tag.closing && WHITESPACE_SENSITIVE_TAGS.has(tag.name)) {
110-
whitespaceSensitiveDepth = Math.max(0, whitespaceSensitiveDepth - 1)
111-
}
112-
normalized.push(normalizeTag(token))
113-
if (!tag.closing && !tag.selfClosing && WHITESPACE_SENSITIVE_TAGS.has(tag.name)) {
114-
whitespaceSensitiveDepth += 1
115-
}
116-
return
70+
const normalizeText = (text: Text, afterBoundary: boolean, beforeBoundary: boolean): void => {
71+
if (text.data.trim() === '') {
72+
if (afterBoundary || beforeBoundary) {
73+
text.remove()
74+
} else {
75+
text.data = ' '
11776
}
77+
return
78+
}
11879

119-
if (token.startsWith('<') || whitespaceSensitiveDepth > 0) {
120-
normalized.push(token)
121-
return
122-
}
80+
let collapsed = text.data.replace(/\s+/g, ' ')
81+
if (afterBoundary) {
82+
collapsed = collapsed.replace(/^ /, '')
83+
}
84+
if (beforeBoundary) {
85+
collapsed = collapsed.replace(/ $/, '')
86+
}
87+
text.data = collapsed
88+
}
12389

124-
const previousToken = tokens[index - 1]
125-
const nextToken = tokens[index + 1]
90+
const normalizeChildren = (parent: Node, whitespaceSensitive: boolean): void => {
91+
const childrenAsParsed = Array.from(parent.childNodes)
12692

127-
if (token.trim() === '') {
128-
if (!isBlockBoundary(previousToken) && !isBlockBoundary(nextToken)) {
129-
normalized.push(' ')
130-
}
93+
childrenAsParsed.forEach((child, index) => {
94+
if (isElement(child)) {
95+
sortAttributes(child)
96+
normalizeChildren(
97+
child,
98+
whitespaceSensitive || WHITESPACE_SENSITIVE_TAGS.has(tagNameOf(child))
99+
)
131100
return
132101
}
133-
134-
let text = token.replace(/\s+/g, ' ')
135-
if (isBlockBoundary(previousToken)) {
136-
text = text.replace(/^ /, '')
137-
}
138-
if (isBlockBoundary(nextToken)) {
139-
text = text.replace(/ $/, '')
102+
if (isText(child) && !whitespaceSensitive) {
103+
normalizeText(
104+
child,
105+
isBlockBoundary(childrenAsParsed[index - 1], parent),
106+
isBlockBoundary(childrenAsParsed[index + 1], parent)
107+
)
140108
}
141-
normalized.push(text)
142109
})
110+
}
143111

144-
return normalized.join('')
112+
export const normalizeHtml = (html: string): string => {
113+
const template = document.createElement('template')
114+
template.innerHTML = html
115+
normalizeChildren(template.content, false)
116+
return template.innerHTML
145117
}

‎test/unit/testHelpers/htmlNormalizer.test.ts‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { normalizeHtml } from '../../testHelpers/html/htmlNormalizer'
22
import {
33
CONTENT_FIELD_WITH_ALL_TAGS,
4-
EXPECTED_CONTENT_FIELD_WITH_ALL_TAGS
4+
SERVER_FORMATTED_CONTENT_FIELD_WITH_ALL_TAGS
55
} from '../../testHelpers/collections/collectionHelper'
66

77
describe('normalizeHtml', () => {
@@ -28,9 +28,29 @@ describe('normalizeHtml', () => {
2828
expect(normalizeHtml('<P CLASS="x">t</P>')).toEqual(normalizeHtml('<p class="x">t</p>'))
2929
})
3030

31-
test('should treat the sent and pretty-printed forms of the featured item fixture as equal', () => {
31+
test('should ignore the order of attributes whose values contain angle brackets', () => {
32+
expect(normalizeHtml('<a title="A > B" href="/example">link</a>')).toEqual(
33+
normalizeHtml('<a href="/example" title="A > B">link</a>')
34+
)
35+
})
36+
37+
test('should ignore the order of attributes quoted with single quotes', () => {
38+
expect(normalizeHtml("<a title='A > B' href='/example'>link</a>")).toEqual(
39+
normalizeHtml('<a href="/example" title="A > B">link</a>')
40+
)
41+
})
42+
43+
test('should ignore equivalent spellings of a void element', () => {
44+
expect(normalizeHtml('<p>a<br>b</p>')).toEqual(normalizeHtml('<p>a<br/>b</p>'))
45+
})
46+
47+
test('should ignore equivalent spellings of an escaped character', () => {
48+
expect(normalizeHtml('<p>a &amp; b</p>')).toEqual(normalizeHtml('<p>a & b</p>'))
49+
})
50+
51+
test('should treat the sent and server-returned forms of the featured item fixture as equal', () => {
3252
expect(normalizeHtml(CONTENT_FIELD_WITH_ALL_TAGS)).toEqual(
33-
normalizeHtml(EXPECTED_CONTENT_FIELD_WITH_ALL_TAGS)
53+
normalizeHtml(SERVER_FORMATTED_CONTENT_FIELD_WITH_ALL_TAGS)
3454
)
3555
})
3656
})
@@ -52,6 +72,12 @@ describe('normalizeHtml', () => {
5272
)
5373
})
5474

75+
test('should not ignore a differing attribute value that contains angle brackets', () => {
76+
expect(normalizeHtml('<a title="A > B" href="/example">link</a>')).not.toEqual(
77+
normalizeHtml('<a title="A > C" href="/example">link</a>')
78+
)
79+
})
80+
5581
test('should not ignore differing structure', () => {
5682
expect(normalizeHtml('<ul><li>a</li><li>b</li></ul>')).not.toEqual(
5783
normalizeHtml('<ul><li>a</li></ul>')
@@ -68,6 +94,12 @@ describe('normalizeHtml', () => {
6894
)
6995
})
7096

97+
test('should preserve the whitespace that separates inline elements in the featured item fixture', () => {
98+
expect(normalizeHtml(CONTENT_FIELD_WITH_ALL_TAGS)).not.toEqual(
99+
normalizeHtml(CONTENT_FIELD_WITH_ALL_TAGS.replace('</strong> <em', '</strong><em'))
100+
)
101+
})
102+
71103
test('should preserve whitespace that separates inline elements', () => {
72104
expect(normalizeHtml('<p><em>a</em> <em>b</em></p>')).not.toEqual(
73105
normalizeHtml('<p><em>a</em><em>b</em></p>')

0 commit comments

Comments
 (0)