fix(core): stop mutating Context's input (#3076)

* add test * stop mutating * does not mutate inputs * cleanup * clone another way * remove temp variable * don't deep clone dom nodes * remove console log * add comment Co-authored-by: Stephen Mathieson <[email protected]> * check if window exists Co-authored-by: Steven Lambert <[email protected]> * clone all at once at first * also allow instanceof HTMLCollection for ie11 Co-authored-by: Steven Lambert <[email protected]> Co-authored-by: Stephen Mathieson <[email protected]> Co-authored-by: Steven Lambert <[email protected]>
dequelabs · Jul 15, 2021 · 5dc34ee · 5dc34ee
1 parent 0865bd7
commit 5dc34ee
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 1 deletion.
diff --git a/lib/core/base/context.js b/lib/core/base/context.js
@@ -7,7 +7,8 @@ import {
   select,
   isNodeInContext,
   nodeSorter,
-  respondable
+  respondable,
+  clone
 } from '../utils';
 
 /**
@@ -230,6 +231,7 @@ function getRootNode({ include, exclude }) {
  * @param {Object} spec Configuration or "specification" object
  */
 export default function Context(spec, flatTree) {
+  spec = clone(spec);
   this.frames = [];
   this.page = typeof spec?.page === 'boolean' ? spec.page : undefined;
   this.initiator = typeof spec?.initiator === 'boolean' ? spec.initiator : true;

diff --git a/lib/core/utils/clone.js b/lib/core/utils/clone.js
@@ -8,6 +8,13 @@ function clone(obj) {
   var index,
     length,
     out = obj;
+  // DOM nodes cannot be cloned.
+  if (
+    (window?.Node && obj instanceof window.Node) || 
+    (window?.HTMLCollection && obj instanceof window.HTMLCollection)
+  ) {
+    return obj;
+  }
 
   if (obj !== null && typeof obj === 'object') {
     if (Array.isArray(obj)) {

diff --git a/test/core/base/context.js b/test/core/base/context.js
@@ -14,6 +14,48 @@ describe('Context', function() {
     fixture.innerHTML = '';
   });
 
+  it('should not mutate exclude in input', function() {
+    fixture.innerHTML = '<div id="foo"></div>';
+    var context = { exclude: [['iframe', '#foo']] };
+    // eslint-disable-next-line no-new
+    new Context(context);
+    assert.deepEqual(context, { exclude: [['iframe', '#foo']] });
+  });
+
+  it('should not mutate its include input', function() {
+    fixture.innerHTML = '<div id="foo"></div>';
+    var context = { include: [['#foo']] };
+    // eslint-disable-next-line no-new
+    new Context(context);
+    assert.deepEqual(context, { include: [['#foo']] });
+  });
+
+  it('should not share memory with complex object', function() {
+    fixture.innerHTML = '<div id="foo"><a href="">Click me</a></div>';
+    var spec = {
+      include: [['#foo'], ['a']],
+      exclude: [['iframe', '#foo2']],
+      size: { width: 100, height: 100 }
+    };
+    var context = new Context(spec);
+    assert.notStrictEqual(spec.include, context.include);
+    spec.include.forEach(function(_, index) {
+      assert.notStrictEqual(spec.include[index], context.include[index]);
+    });
+    assert.notStrictEqual(spec.exclude, context.exclude);
+    spec.exclude.forEach(function(_, index) {
+      assert.notStrictEqual(spec.exclude[index], context.exclude[index]);
+    });
+    assert.notStrictEqual(spec.size, context.size);
+  });
+
+  it('should not share memory with simple array', function() {
+    fixture.innerHTML = '<div id="foo"></div>';
+    var spec = ['#foo'];
+    var context = new Context(spec);
+    assert.notStrictEqual(spec, context.include);
+  });
+
   describe('include', function() {
     it('should accept a single selector', function() {
       fixture.innerHTML = '<div id="foo"></div>';