Skip to content

Commit 4c9e73c

Browse files
Release 4.4.0-protobi.9: Fix VML comment protection parsing
Fixes critical bug where comment protection properties (locked, lockText) and positioning (editAs) were not being parsed correctly during file reading. Root cause: SAX parser strips 'x:' namespace prefix but keeps 'v:' prefix. Updated 5 VML xform classes to use unprefixed tag names for parsing while still writing prefixed XML for Excel compatibility. Testing: 1091/1091 tests passing (fixed 2 previously failing tests) Files changed: - FORK.md: Added detailed bug fix documentation - README.md: Updated release notes - 5 VML xform files: Fixed namespace prefix handling - package.json, package-lock.json: Version bump to 4.4.0-protobi.9
1 parent f5a949c commit 4c9e73c

9 files changed

Lines changed: 72 additions & 38 deletions

File tree

FORK.md

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ Implements complete round-trip preservation for Excel files containing pivot tab
7373
- Relationship mappings preserved correctly
7474

7575
**Testing:**
76-
-884/884 unit tests passing
76+
-1091/1091 tests passing (unit + integration + end-to-end)
7777
- ✅ Round-trip test with real-world pivot table/chart files
7878
- ✅ Excel opens files without corruption warnings
7979
- ✅ Pivot tables remain functional after round-trip
@@ -85,6 +85,42 @@ Implements complete round-trip preservation for Excel files containing pivot tab
8585
- This implementation enables preservation, not programmatic access
8686
- Hybrid approach: pivot tables can be created OR preserved, not both simultaneously
8787

88+
---
89+
90+
**Bug Fix: Cell Comment/Note Protection Properties** (2026-02-02)
91+
92+
Fixes a critical bug where comment protection properties (`locked`, `lockText`) and positioning (`editAs`) were not being parsed correctly during file reading, causing them to return `null` instead of their actual values.
93+
94+
**Root Cause:**
95+
- SAX XML parser strips `x:` namespace prefix (Excel namespace) but preserves `v:` prefix (VML namespace)
96+
- VML xform classes were using prefixed tag names (`x:Locked`, `x:ClientData`) in their maps
97+
- Parser delivered unprefixed tag names (`Locked`, `ClientData`), causing lookup failures
98+
99+
**The Fix:**
100+
- Updated 5 VML xform classes to use unprefixed tag names for parsing
101+
- Render methods still write prefixed XML for Excel compatibility
102+
- Added comments documenting the SAX parser behavior
103+
104+
**Files Modified:**
105+
- `lib/xlsx/xform/comment/vml-client-data-xform.js`
106+
- `lib/xlsx/xform/comment/vml-shape-xform.js`
107+
- `lib/xlsx/xform/comment/vml-anchor-xform.js`
108+
- `lib/xlsx/xform/comment/style/vml-protection-xform.js`
109+
- `lib/xlsx/xform/comment/style/vml-position-xform.js`
110+
111+
**Testing:**
112+
- ✅ 1091/1091 tests passing (vs 1089 passing before fix)
113+
- ✅ Fixed 2 previously failing integration tests:
114+
- "writes notes" - comment protection properties now round-trip correctly
115+
- "Cell annotation supports setting margins and protection properties"
116+
- ✅ Verified this bug also exists in upstream ExcelJS (not fixed there yet)
117+
118+
**Impact:**
119+
- Comment protection properties (`note.protection.locked`, `note.protection.lockText`) now parse correctly
120+
- Comment margins (`note.margins`) now preserve correctly during round-trip
121+
- Comment positioning (`note.editAs`) now reads back correctly
122+
- Excel files with protected comments can now be read and written without losing properties
123+
88124
**Related Issues:**
89125
- Inspired by [ExcelTS Issue #41](https://github.com/cjnoname/excelts/issues/41)
90126
- Addresses Excel corruption: "Removed Part: /xl/pivotTables/pivotTable1.xml"

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,15 @@ See [FORK.md Release History](FORK.md#fork-release-history) for details.
5959

6060
## Fork Release Notes
6161

62-
**4.4.0-protobi.9** (February 2026) - Pivot Table & Chart Round-Trip Preservation
62+
**4.4.0-protobi.9** (February 2026) - Pivot Table & Chart Round-Trip Preservation + Critical Bug Fixes
6363
- **Round-trip preservation for pivot tables and charts** - Read Excel files with existing pivot tables and charts, write them back without corruption
6464
- Hybrid preservation approach: stores raw XML while extracting minimal metadata for structural integrity
6565
- Preserves pivot table cache definitions, cache records, and relationships
6666
- Preserves charts, chart styles, and chart colors
6767
- Preserves row heights (dyDescent attribute) accurately
6868
- Fixes Excel corruption warnings when round-tripping files with pivot tables/charts
69-
- All 884 unit tests passing
69+
- **Fixes cell comment/note protection properties parsing** - Comment protection (locked/lockText) and margins now correctly round-trip (fixes upstream bug present in base ExcelJS)
70+
- All 1091 tests passing
7071

7172
**4.4.0-protobi.8** (January 2026)
7273
- Form Control Checkbox support

lib/xlsx/xform/comment/style/vml-position-xform.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ class VmlPositionXform extends BaseXform {
1212

1313
render(xmlStream, model, type) {
1414
if (model === type[2]) {
15-
xmlStream.leafNode(this.tag);
16-
} else if (this.tag === 'x:SizeWithCells' && model === type[1]) {
17-
xmlStream.leafNode(this.tag);
15+
xmlStream.leafNode(`x:${this.tag}`);
16+
} else if (this.tag === 'SizeWithCells' && model === type[1]) {
17+
xmlStream.leafNode(`x:${this.tag}`);
1818
}
1919
}
2020

lib/xlsx/xform/comment/style/vml-protection-xform.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class VmlProtectionXform extends BaseXform {
1111
}
1212

1313
render(xmlStream, model) {
14-
xmlStream.leafNode(this.tag, null, model);
14+
xmlStream.leafNode(`x:${this.tag}`, null, model);
1515
}
1616

1717
parseOpen(node) {

lib/xlsx/xform/comment/vml-anchor-xform.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const BaseXform = require('../base-xform');
33
// render the triangle in the cell for the comment
44
class VmlAnchorXform extends BaseXform {
55
get tag() {
6-
return 'x:Anchor';
6+
return 'Anchor'; // SAX parser strips 'x:' prefix
77
}
88

99
getAnchorRect(anchor) {
@@ -35,7 +35,7 @@ class VmlAnchorXform extends BaseXform {
3535
? this.getAnchorRect(model.anchor)
3636
: this.getDefaultRect(model.refAddress);
3737

38-
xmlStream.leafNode('x:Anchor', null, rect.join(', '));
38+
xmlStream.leafNode('x:Anchor', null, rect.join(', ')); // Always write with 'x:' prefix
3939
}
4040

4141
parseOpen(node) {

lib/xlsx/xform/comment/vml-client-data-xform.js

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,29 @@ const POSITION_TYPE = ['twoCells', 'oneCells', 'absolute'];
99
class VmlClientDataXform extends BaseXform {
1010
constructor() {
1111
super();
12+
// SAX parser strips 'x:' prefix from Excel namespace elements
1213
this.map = {
13-
'x:Anchor': new VmlAnchorXform(),
14-
'x:Locked': new VmlProtectionXform({tag: 'x:Locked'}),
15-
'x:LockText': new VmlProtectionXform({tag: 'x:LockText'}),
16-
'x:SizeWithCells': new VmlPositionXform({tag: 'x:SizeWithCells'}),
17-
'x:MoveWithCells': new VmlPositionXform({tag: 'x:MoveWithCells'}),
14+
Anchor: new VmlAnchorXform(),
15+
Locked: new VmlProtectionXform({tag: 'Locked'}),
16+
LockText: new VmlProtectionXform({tag: 'LockText'}),
17+
SizeWithCells: new VmlPositionXform({tag: 'SizeWithCells'}),
18+
MoveWithCells: new VmlPositionXform({tag: 'MoveWithCells'}),
1819
};
1920
}
2021

2122
get tag() {
22-
return 'x:ClientData';
23+
return 'ClientData'; // SAX parser strips 'x:' prefix
2324
}
2425

2526
render(xmlStream, model) {
2627
const {protection, editAs} = model.note;
27-
xmlStream.openNode(this.tag, {ObjectType: 'Note'});
28-
this.map['x:MoveWithCells'].render(xmlStream, editAs, POSITION_TYPE);
29-
this.map['x:SizeWithCells'].render(xmlStream, editAs, POSITION_TYPE);
30-
this.map['x:Anchor'].render(xmlStream, model);
31-
this.map['x:Locked'].render(xmlStream, protection.locked);
28+
xmlStream.openNode('x:ClientData', {ObjectType: 'Note'}); // Write with prefix
29+
this.map.MoveWithCells.render(xmlStream, editAs, POSITION_TYPE);
30+
this.map.SizeWithCells.render(xmlStream, editAs, POSITION_TYPE);
31+
this.map.Anchor.render(xmlStream, model);
32+
this.map.Locked.render(xmlStream, protection.locked);
3233
xmlStream.leafNode('x:AutoFill', null, 'False');
33-
this.map['x:LockText'].render(xmlStream, protection.lockText);
34+
this.map.LockText.render(xmlStream, protection.lockText);
3435
xmlStream.leafNode('x:Row', null, model.refAddress.row - 1);
3536
xmlStream.leafNode('x:Column', null, model.refAddress.col - 1);
3637
xmlStream.closeNode();
@@ -79,16 +80,12 @@ class VmlClientDataXform extends BaseXform {
7980
}
8081

8182
normalizeModel() {
82-
const position = Object.assign(
83-
{},
84-
this.map['x:MoveWithCells'].model,
85-
this.map['x:SizeWithCells'].model
86-
);
83+
const position = Object.assign({}, this.map.MoveWithCells.model, this.map.SizeWithCells.model);
8784
const len = Object.keys(position).length;
8885
this.model.editAs = POSITION_TYPE[len];
89-
this.model.anchor = this.map['x:Anchor'].text;
90-
this.model.protection.locked = this.map['x:Locked'].text;
91-
this.model.protection.lockText = this.map['x:LockText'].text;
86+
this.model.anchor = this.map.Anchor.text;
87+
this.model.protection.locked = this.map.Locked.text;
88+
this.model.protection.lockText = this.map.LockText.text;
9289
}
9390
}
9491

lib/xlsx/xform/comment/vml-shape-xform.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@ const VmlClientDataXform = require('./vml-client-data-xform');
55
class VmlShapeXform extends BaseXform {
66
constructor() {
77
super();
8+
// SAX parser strips 'x:' prefix but keeps 'v:' and 'o:' prefixes
89
this.map = {
910
'v:textbox': new VmlTextboxXform(),
10-
'x:ClientData': new VmlClientDataXform(),
11+
ClientData: new VmlClientDataXform(),
1112
};
1213
}
1314

@@ -22,7 +23,7 @@ class VmlShapeXform extends BaseXform {
2223
xmlStream.leafNode('v:shadow', {color: 'none [81]', obscured: 't'});
2324
xmlStream.leafNode('v:path', {'o:connecttype': 'none'});
2425
this.map['v:textbox'].render(xmlStream, model);
25-
this.map['x:ClientData'].render(xmlStream, model);
26+
this.map.ClientData.render(xmlStream, model);
2627

2728
xmlStream.closeNode();
2829
}
@@ -71,10 +72,9 @@ class VmlShapeXform extends BaseXform {
7172
switch (name) {
7273
case this.tag:
7374
this.model.margins.inset = this.map['v:textbox'].model && this.map['v:textbox'].model.inset;
74-
this.model.protection =
75-
this.map['x:ClientData'].model && this.map['x:ClientData'].model.protection;
76-
this.model.anchor = this.map['x:ClientData'].model && this.map['x:ClientData'].model.anchor;
77-
this.model.editAs = this.map['x:ClientData'].model && this.map['x:ClientData'].model.editAs;
75+
this.model.protection = this.map.ClientData.model && this.map.ClientData.model.protection;
76+
this.model.anchor = this.map.ClientData.model && this.map.ClientData.model.anchor;
77+
this.model.editAs = this.map.ClientData.model && this.map.ClientData.model.editAs;
7878
return false;
7979
default:
8080
return true;

package-lock.json

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

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@protobi/exceljs",
3-
"version": "4.4.0-protobi.8",
3+
"version": "4.4.0-protobi.9",
44
"description": "Excel Workbook Manager - Temporary fork with pivot table enhancements and bug fixes pending upstream merge",
55
"private": false,
66
"license": "MIT",

0 commit comments

Comments
 (0)