Follow-up comments on PR #12344

src/core/core_utils.js

@@ -165,6 +165,26 @@ function isWhiteSpace(ch) {
   return ch === 0x20 || ch === 0x09 || ch === 0x0d || ch === 0x0a;
 }
 
+/**
+ * AcroForm field names use an array like notation to refer to
+ * repeated XFA elements e.g. foo.bar[nnn].
+ * see: XFA Spec Chapter 3 - Repeated Elements
+ *
+ * @param {string} path - XFA path name.
+ * @returns {Array} - Array of Objects with the name and pos of
+ * each part of the path.
+ */
+function parseXFAPath(path) {
+  const positionPattern = /(.+)\[([0-9]+)\]$/;
+  return path.split(".").map(component => {
+    const m = component.match(positionPattern);
+    if (m) {
+      return { name: m[1], pos: parseInt(m[2], 10) };
+    }
+    return { name: component, pos: 0 };
+  });
+}
+
 export {
   getLookupTableFactory,
   MissingDataException,
@@ -173,6 +193,7 @@ export {
   getInheritableProperty,
   toRomanNumerals,
   log2,
+  parseXFAPath,
   readInt8,
   readUint16,
   readUint32,

src/core/worker.js

@@ -589,13 +589,13 @@ class WorkerMessageHandler {
         }
         xref.resetNewRef();
 
-        return incrementalUpdate(
-          stream.bytes,
-          newXrefInfo,
+        return incrementalUpdate({
+          originalData: stream.bytes,
+          xrefInfo: newXrefInfo,
           newRefs,
           xref,
-          xfaDatasets
-        );
+          datasetsRef: xfaDatasets,
+        });
       });
     });
 

src/core/writer.js

@@ -14,15 +14,11 @@
  */
 /* eslint no-var: error */
 
-import {
-  bytesToString,
-  escapeString,
-  parseXFAPath,
-  warn,
-} from "../shared/util.js";
+import { bytesToString, escapeString, warn } from "../shared/util.js";
 import { Dict, isDict, isName, isRef, isStream, Name } from "./primitives.js";
 import { SimpleDOMNode, SimpleXMLParser } from "../shared/xml_parser.js";
 import { calculateMD5 } from "./crypto.js";
+import { parseXFAPath } from "./core_utils.js";
 
 function writeDict(dict, buffer, transform) {
   buffer.push("<<");
@@ -175,7 +171,13 @@ function updateXFA(datasetsRef, newRefs, xref) {
   newRefs.push({ ref: datasetsRef, data });
 }
 
-function incrementalUpdate(originalData, xrefInfo, newRefs, xref, datasetsRef) {
+function incrementalUpdate({
+  originalData,
+  xrefInfo,
+  newRefs,
+  xref = null,
+  datasetsRef = null,
+}) {
   updateXFA(datasetsRef, newRefs, xref);
 
   const newXref = new Dict(null);

src/shared/util.js

@@ -910,26 +910,6 @@ const createObjectURL = (function createObjectURLClosure() {
   };
 })();
 
-/**
- * AcroForm field names use an array like notation to refer to
- * repeated XFA elements e.g. foo.bar[nnn].
- * see: XFA Spec Chapter 3 - Repeated Elements
- *
- * @param {string} path - XFA path name.
- * @returns {Array} - Array of Objects with the name and pos of
- * each part of the path.
- */
-function parseXFAPath(path) {
-  const positionPattern = /(.+)\[([0-9]+)\]$/;
-  return path.split(".").map(component => {
-    const m = component.match(positionPattern);
-    if (m) {
-      return { name: m[1], pos: parseInt(m[2], 10) };
-    }
-    return { name: component, pos: 0 };
-  });
-}
-
 const XMLEntities = {
   /* < */ 0x3c: "&lt;",
   /* > */ 0x3e: "&gt;",
@@ -1027,7 +1007,6 @@ export {
   createValidAbsoluteUrl,
   IsLittleEndianCached,
   IsEvalSupportedCached,
-  parseXFAPath,
   removeNullCharacters,
   setVerbosityLevel,
   shadow,

src/shared/xml_parser.js

@@ -329,6 +329,18 @@ class SimpleDOMNode {
     return this.childNodes && this.childNodes.length > 0;
   }
 
+  /**
+   * Search a node in the tree with the given path
+   * foo.bar[nnn], i.e. find the nnn-th node named
+   * bar under a node named foo.
+   *
+   * @param {Array} paths - an array of objects as
+   * returned by {parseXFAPath}.
+   * @param {number} pos - the current position in
+   * the paths array.
+   * @returns {SimpleDOMNode} The node corresponding
+   * to the path or null if not found.
+   */
   searchNode(paths, pos) {
     if (pos >= paths.length) {
       return this;

test/unit/core_utils_spec.js

@@ -18,6 +18,7 @@ import {
   getInheritableProperty,
   isWhiteSpace,
   log2,
+  parseXFAPath,
   toRomanNumerals,
 } from "../../src/core/core_utils.js";
 import { XRefMock } from "./test_utils.js";
@@ -211,4 +212,18 @@ describe("core_utils", function () {
       expect(isWhiteSpace(undefined)).toEqual(false);
     });
   });
+
+  describe("parseXFAPath", function () {
+    it("should get a correctly parsed path", function () {
+      const path = "foo.bar[12].oof[3].rab.FOO[123].BAR[456]";
+      expect(parseXFAPath(path)).toEqual([
+        { name: "foo", pos: 0 },
+        { name: "bar", pos: 12 },
+        { name: "oof", pos: 3 },
+        { name: "rab", pos: 0 },
+        { name: "FOO", pos: 123 },
+        { name: "BAR", pos: 456 },
+      ]);
+    });
+  });
 });

test/unit/util_spec.js

@@ -25,7 +25,6 @@ import {
   isNum,
   isSameOrigin,
   isString,
-  parseXFAPath,
   removeNullCharacters,
   string32,
   stringToBytes,
@@ -334,20 +333,6 @@ describe("util", function () {
     });
   });
 
-  describe("parseXFAPath", function () {
-    it("should get a correctly parsed path", function () {
-      const path = "foo.bar[12].oof[3].rab.FOO[123].BAR[456]";
-      expect(parseXFAPath(path)).toEqual([
-        { name: "foo", pos: 0 },
-        { name: "bar", pos: 12 },
-        { name: "oof", pos: 3 },
-        { name: "rab", pos: 0 },
-        { name: "FOO", pos: 123 },
-        { name: "BAR", pos: 456 },
-      ]);
-    });
-  });
-
   describe("encodeToXmlString", function () {
     it("should get a correctly encoded string with some entities", function () {
       const str = "\"\u0397ell😂' & <W😂rld>";

test/unit/writer_spec.js

@@ -37,7 +37,7 @@ describe("Writer", function () {
         info: {},
       };
 
-      let data = incrementalUpdate(originalData, xrefInfo, newRefs, null, null);
+      let data = incrementalUpdate({ originalData, xrefInfo, newRefs });
       data = bytesToString(data);
 
       const expected =

test/unit/xml_spec.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { parseXFAPath } from "../../src/shared/util.js";
+import { parseXFAPath } from "../../src/core/core_utils.js";
 import { SimpleXMLParser } from "../../src/shared/xml_parser.js";
 
 describe("XML", function () {
@@ -69,7 +69,7 @@ describe("XML", function () {
     });
 
     it("should dump a xml tree", function () {
-      let xml = `
+      const xml = `
       <a>
           <b>
               <c a="123"/>
@@ -87,9 +87,7 @@ describe("XML", function () {
           <h>
               <i/>
               <j/>
-              <k>
-                  W&#x1F602;rld
-                  <g a="654"/>
+              <k>&#xA;W&#x1F602;rld&#xA;<g a="654"/>
               </k>
           </h>
           <b>
@@ -98,13 +96,14 @@ describe("XML", function () {
               <g a="121110"/>
           </b>
       </a>`;
-      xml = xml.replace(/\s+/g, "");
       const root = new SimpleXMLParser(true).parseFromString(xml)
         .documentElement;
       const buffer = [];
       root.dump(buffer);
 
-      expect(buffer.join("").replace(/\s+/g, "")).toEqual(xml);
+      expect(buffer.join("").replace(/\s+/g, "")).toEqual(
+        xml.replace(/\s+/g, "")
+      );
     });
   });
 });