From 08cebe7985079ef4f886f25952df27fe868f8e6f Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 25 Feb 2026 02:13:54 +0100 Subject: [PATCH 1/4] fix: parse HTML instead of XML in SvgCanvas2D.convertHtml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The method was incorrectly using `parseXml` (XML parser) instead of `DOMParser` with `text/html`. This bug was introduced in commit 2f4bb076 while trying to share code between methods — the code wasn't actually the same. - Use `DOMParser.parseFromString(val, 'text/html')` for correct HTML parsing - Add tests for `convertHtml` covering plain text, nested HTML, malformed HTML, empty input, and body tag extraction - Improve JSDoc of the `text` method --- .../__tests__/view/canvas/SvgCanvas2D.test.ts | 68 +++++++++++++++++++ packages/core/src/view/canvas/SvgCanvas2D.ts | 14 ++-- 2 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts diff --git a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts new file mode 100644 index 0000000000..df7b0a0ee7 --- /dev/null +++ b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts @@ -0,0 +1,68 @@ +/* +Copyright 2026-present The maxGraph project Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { describe, expect, test } from '@jest/globals'; + +import SvgCanvas2D from '../../../src/view/canvas/SvgCanvas2D.js'; +import { NS_SVG } from '../../../src/util/Constants.js'; + +describe('SvgCanvas2D.convertHtml', () => { + // Helper to create a dummy SVG root + function createSvgCanvas2D() { + return new SvgCanvas2D(document.createElementNS(NS_SVG, 'svg'), true); + } + + test('returns plain text unchanged if not valid HTML', () => { + const canvas = createSvgCanvas2D(); + const input = 'plain text'; + expect(canvas.convertHtml(input)).toBe('plain text'); + }); + + test('handles HTML with body tag', () => { + const canvas = createSvgCanvas2D(); + const input = '

foo

'; + expect(canvas.convertHtml(input)).toBe('

foo

'); + }); + + test('handles HTML with attributes on body', () => { + const canvas = createSvgCanvas2D(); + const input = 'bar'; + expect(canvas.convertHtml(input)).toBe('bar'); + }); + + test('returns empty string for empty input', () => { + const canvas = createSvgCanvas2D(); + expect(canvas.convertHtml('')).toBe(''); + }); + + test('handles nested HTML', () => { + const canvas = createSvgCanvas2D(); + const input = '
deep
'; + expect(canvas.convertHtml(input)).toBe('
deep
'); + }); + + test('handles malformed HTML gracefully', () => { + const canvas = createSvgCanvas2D(); + const input = '
broken'; + expect(canvas.convertHtml(input)).toBe('
broken
'); + }); + + test('handles HTML with whitespace', () => { + const canvas = createSvgCanvas2D(); + const input = '
spaced
'; + expect(canvas.convertHtml(input).trim()).toBe('
spaced
'); + }); +}); diff --git a/packages/core/src/view/canvas/SvgCanvas2D.ts b/packages/core/src/view/canvas/SvgCanvas2D.ts index 4872a18776..9d76d36ee3 100644 --- a/packages/core/src/view/canvas/SvgCanvas2D.ts +++ b/packages/core/src/view/canvas/SvgCanvas2D.ts @@ -1087,9 +1087,10 @@ class SvgCanvas2D extends AbstractCanvas2D { * Converts the given HTML string to XHTML. */ convertHtml(val: string) { - const doc = parseXml(val); + const doc = new DOMParser().parseFromString(val, 'text/html'); - if (doc != null) { + // the jsdoc of DOMParser.parseFromString says the returned value is never null, but keep the check (it comes from mxGraph) for now until we get more feedback on this + if (doc) { val = new XMLSerializer().serializeToString(doc.body); // Extracts body content from DOM @@ -1383,10 +1384,11 @@ class SvgCanvas2D extends AbstractCanvas2D { } /** - * Paints the given text. Possible values for format are empty string for plain - * text and html for HTML markup. Note that HTML markup is only supported if - * foreignObject is supported and is true. (This means IE9 and later - * does currently not support HTML text as part of shapes.) + * Paints the given text. + * + * Possible values for format are empty string for plain text and HTML for HTML markup. + * + * Note that HTML markup is only supported if `foreignObject` is supported and {@link foEnabled} is `true`. */ text( x: number, From b2b6799139e1dd9e651df74ed567db700c59cca2 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 25 Feb 2026 02:26:50 +0100 Subject: [PATCH 2/4] test: simplify --- .../core/__tests__/view/canvas/SvgCanvas2D.test.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts index df7b0a0ee7..82161dbdd2 100644 --- a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts +++ b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts @@ -15,16 +15,13 @@ limitations under the License. */ import { describe, expect, test } from '@jest/globals'; +import { SvgCanvas2D, constants } from '../../../src'; -import SvgCanvas2D from '../../../src/view/canvas/SvgCanvas2D.js'; -import { NS_SVG } from '../../../src/util/Constants.js'; +function createSvgCanvas2D() { + return new SvgCanvas2D(document.createElementNS(constants.NS_SVG, 'svg'), true); +} describe('SvgCanvas2D.convertHtml', () => { - // Helper to create a dummy SVG root - function createSvgCanvas2D() { - return new SvgCanvas2D(document.createElementNS(NS_SVG, 'svg'), true); - } - test('returns plain text unchanged if not valid HTML', () => { const canvas = createSvgCanvas2D(); const input = 'plain text'; From de64192d67f478f7a94fe6a7b3111a69f9426046 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 25 Feb 2026 11:43:13 +0100 Subject: [PATCH 3/4] test: simplify by using test.each --- CLAUDE.md | 19 +++++++ .../__tests__/view/canvas/SvgCanvas2D.test.ts | 56 ++++++------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3051156b29..4cfebde2fa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -352,6 +352,25 @@ Styles are defined as objects conforming to `CellStyle` type. See `packages/core - Tests are in `packages/core/__tests__/` mirroring `src/` structure - Use `@swc/jest` for fast TypeScript compilation - Import paths in tests should omit `.js` extension (handled by moduleNameMapper) +- When multiple tests share the same structure and only differ by input/expected data, use `test.each` (or `it.each`) to avoid repetition and put focus on the tested use cases: + +```typescript +// Good - data-driven tests with test.each +test.each([ + ['description of case 1', input1, expected1], + ['description of case 2', input2, expected2], +])('%s', (_description, input, expected) => { + expect(myFunction(input)).toBe(expected); +}); + +// Bad - repetitive tests with identical structure +test('case 1', () => { + expect(myFunction(input1)).toBe(expected1); +}); +test('case 2', () => { + expect(myFunction(input2)).toBe(expected2); +}); +``` ## Commit Message Style diff --git a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts index 82161dbdd2..5f82f82c4d 100644 --- a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts +++ b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts @@ -22,44 +22,24 @@ function createSvgCanvas2D() { } describe('SvgCanvas2D.convertHtml', () => { - test('returns plain text unchanged if not valid HTML', () => { + test.each([ + ['plain text unchanged if not valid HTML', 'plain text', 'plain text'], + ['HTML with body tag', '

foo

', '

foo

'], + [ + 'HTML with attributes on body', + 'bar', + 'bar', + ], + ['empty string for empty input', '', ''], + [ + 'nested HTML', + '
deep
', + '
deep
', + ], + ['malformed HTML gracefully', '
broken', '
broken
'], + ['HTML with whitespace', '
spaced
', '
spaced
'], + ])('%s', (_description, input, expected) => { const canvas = createSvgCanvas2D(); - const input = 'plain text'; - expect(canvas.convertHtml(input)).toBe('plain text'); - }); - - test('handles HTML with body tag', () => { - const canvas = createSvgCanvas2D(); - const input = '

foo

'; - expect(canvas.convertHtml(input)).toBe('

foo

'); - }); - - test('handles HTML with attributes on body', () => { - const canvas = createSvgCanvas2D(); - const input = 'bar'; - expect(canvas.convertHtml(input)).toBe('bar'); - }); - - test('returns empty string for empty input', () => { - const canvas = createSvgCanvas2D(); - expect(canvas.convertHtml('')).toBe(''); - }); - - test('handles nested HTML', () => { - const canvas = createSvgCanvas2D(); - const input = '
deep
'; - expect(canvas.convertHtml(input)).toBe('
deep
'); - }); - - test('handles malformed HTML gracefully', () => { - const canvas = createSvgCanvas2D(); - const input = '
broken'; - expect(canvas.convertHtml(input)).toBe('
broken
'); - }); - - test('handles HTML with whitespace', () => { - const canvas = createSvgCanvas2D(); - const input = '
spaced
'; - expect(canvas.convertHtml(input).trim()).toBe('
spaced
'); + expect(canvas.convertHtml(input).trim()).toBe(expected); }); }); From cde3bc804780bad4293cfcedec0c985ba8fabeb4 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 25 Feb 2026 15:32:26 +0100 Subject: [PATCH 4/4] test: check exact returned value, do not process (trim) to make the test clearer --- packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts index 5f82f82c4d..d70e20bbb7 100644 --- a/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts +++ b/packages/core/__tests__/view/canvas/SvgCanvas2D.test.ts @@ -37,9 +37,13 @@ describe('SvgCanvas2D.convertHtml', () => { '
deep
', ], ['malformed HTML gracefully', '
broken', '
broken
'], - ['HTML with whitespace', '
spaced
', '
spaced
'], + [ + 'HTML with whitespace, trim only leading whitespace', + '
spaced
', + '
spaced
', + ], ])('%s', (_description, input, expected) => { const canvas = createSvgCanvas2D(); - expect(canvas.convertHtml(input).trim()).toBe(expected); + expect(canvas.convertHtml(input)).toBe(expected); }); });