Skip to content

fix: keep CharacterData nodeValue and data in sync#990

Merged
karfau merged 5 commits into
xmldom:masterfrom
stevenobiajulu:fix/characterdata-nodevalue-data-sync
Apr 18, 2026
Merged

fix: keep CharacterData nodeValue and data in sync#990
karfau merged 5 commits into
xmldom:masterfrom
stevenobiajulu:fix/characterdata-nodevalue-data-sync

Conversation

@stevenobiajulu

@stevenobiajulu stevenobiajulu commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #989.

Problem

CharacterData stores text in two plain properties — data and nodeValue — that are not kept in sync under direct property assignment:

const text = doc.documentElement.firstChild;
text.nodeValue = 'Changed';
text.data;  // still 'Hello'
new XMLSerializer().serializeToString(doc);  // '<root>Hello</root>'

XMLSerializer reads node.data, so changes via nodeValue are silently lost on serialization. The reverse (node.data = 'text' leaving nodeValue stale) also applies.

Built-in mutation methods (appendData, replaceData, splitText) are unaffected — they explicitly assign both properties. The issue is only with direct property assignment.

Fix

Add Object.defineProperty accessors for data and nodeValue on CharacterData.prototype, inside the existing try { if (Object.defineProperty) { ... } } block:

  • data is the canonical store (backed by _data); the getter normalizes null/undefined to '', and the setter updates length for string assignments
  • nodeValue is a thin alias that delegates to data

This follows the same pattern already used in the codebase for textContent and LiveNodeList.length.

Existing mutation methods are unchanged — their this.nodeValue = this.data = text assignments now flow through the setters, which is redundant but correct and avoids any behavior change for environments without Object.defineProperty.

Tests

New tests in test/dom/character-data.test.js, covering:

  • nodeValue assignment updates data and length (Text, Comment, CDATASection)
  • data assignment updates nodeValue and length (Text, Comment, CDATASection)
  • XMLSerializer output reflects both assignment paths per node type
  • null values stay in sync between data and nodeValue
  • replaceData and appendData still work correctly
  • cloneNode preserves data/nodeValue/length
  • splitText preserves sync on both halves
  • ProcessingInstruction nodeValue/data sync and serializer output

Spec reference

WHATWG DOM Living Standard §4.10 — for CharacterData nodes, the nodeValue getter and setter must be equivalent to data.

Environments without Object.defineProperty keep existing behavior; this PR does not add a separate fallback.

Related to #42.

Add Object.defineProperty accessors for data and nodeValue on
CharacterData.prototype so that direct property assignment keeps
both properties and length synchronized.

Fixes xmldom#989
Store the raw value and only update length for strings.
This preserves existing behavior for internal code paths
that pass undefined (e.g. createProcessingInstruction).
@codecov

codecov Bot commented Apr 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.70%. Comparing base (bf396a5) to head (e578552).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #990      +/-   ##
==========================================
+ Coverage   96.25%   96.70%   +0.45%     
==========================================
  Files           8        8              
  Lines        2296     2307      +11     
  Branches      608      610       +2     
==========================================
+ Hits         2210     2231      +21     
+ Misses         86       76      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stevenobiajulu stevenobiajulu marked this pull request as ready for review April 7, 2026 04:27
@stevenobiajulu stevenobiajulu marked this pull request as draft April 7, 2026 04:30
Set length to 0 for non-string values instead of leaving it stale.
@stevenobiajulu stevenobiajulu marked this pull request as ready for review April 7, 2026 04:31
…odevalue-data-sync

# Conflicts:
#	lib/dom.js
@karfau karfau merged commit 36df573 into xmldom:master Apr 18, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CharacterData: direct property assignment to nodeValue or data does not stay in sync

2 participants