Skip to content

Commit

Permalink
fix(Canvas): Safeguard from multiple initialization (#7776)
Browse files Browse the repository at this point in the history
* fix(Canvas): safegurad double canvas wrapping

* data fabric attr

* remove redundat func signature `getTopContext`

* Update tests
  • Loading branch information
ShaMan123 authored Mar 30, 2022
1 parent ed00026 commit ec8d379
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 26 deletions.
14 changes: 5 additions & 9 deletions src/canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -975,22 +975,14 @@
this.upperCanvasEl = upperCanvasEl;
}
fabric.util.addClass(upperCanvasEl, 'upper-canvas ' + lowerCanvasClass);

this.upperCanvasEl.setAttribute('data-fabric', 'top');
this.wrapperEl.appendChild(upperCanvasEl);

this._copyCanvasStyle(lowerCanvasEl, upperCanvasEl);
this._applyCanvasStyle(upperCanvasEl);
this.contextTop = upperCanvasEl.getContext('2d');
},

/**
* Returns context of top canvas where interactions are drawn
* @returns {CanvasRenderingContext2D}
*/
getTopContext: function () {
return this.contextTop;
},

/**
* @private
*/
Expand All @@ -1005,9 +997,13 @@
* @private
*/
_initWrapperElement: function () {
if (this.wrapperEl) {
return;
}
this.wrapperEl = fabric.util.wrapElement(this.lowerCanvasEl, 'div', {
'class': this.containerClass
});
this.wrapperEl.setAttribute('data-fabric', 'wrapper');
fabric.util.setStyle(this.wrapperEl, {
width: this.width + 'px',
height: this.height + 'px',
Expand Down
8 changes: 7 additions & 1 deletion src/static_canvas.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,13 @@
else {
this.lowerCanvasEl = fabric.util.getById(canvasEl) || this._createCanvasElement();
}

if (this.lowerCanvasEl.hasAttribute('data-fabric')) {
/* _DEV_MODE_START_ */
throw new Error('fabric.js: trying to initialize a canvas that has already been initialized');
/* _DEV_MODE_END_ */
}
fabric.util.addClass(this.lowerCanvasEl, 'lower-canvas');
this.lowerCanvasEl.setAttribute('data-fabric', 'main');
this._originalCanvasStyle = this.lowerCanvasEl.style;
if (this.interactive) {
this._applyCanvasStyle(this.lowerCanvasEl);
Expand Down Expand Up @@ -1571,6 +1576,7 @@
this.contextContainer = null;
// restore canvas style
this.lowerCanvasEl.classList.remove('lower-canvas');
this.lowerCanvasEl.removeAttribute('data-fabric');
fabric.util.setStyle(this.lowerCanvasEl, this._originalCanvasStyle);
delete this._originalCanvasStyle;
// restore canvas size to original size in case retina scaling was applied
Expand Down
9 changes: 9 additions & 0 deletions test/unit/canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@
}
});

QUnit.test('prevent multiple canvas initialization', function (assert) {
var canvas = new fabric.Canvas();
assert.ok(canvas.lowerCanvasEl);
assert.throws(() => new fabric.Canvas(canvas.lowerCanvasEl));
});

QUnit.test('initialProperties', function(assert) {
assert.ok('backgroundColor' in canvas);
assert.equal(canvas.includeDefaultValues, true);
Expand Down Expand Up @@ -207,6 +213,9 @@

QUnit.test('_initInteractive', function(assert) {
assert.ok(typeof canvas._initInteractive === 'function');
assert.equal(canvas.lowerCanvasEl.getAttribute('data-fabric'), 'main', 'el should be marked by canvas init');
assert.equal(canvas.upperCanvasEl.getAttribute('data-fabric'), 'top', 'el should be marked by canvas init');
assert.equal(canvas.wrapperEl.getAttribute('data-fabric'), 'wrapper', 'el should be marked by canvas init');
});

QUnit.test('renderTop', function(assert) {
Expand Down
32 changes: 16 additions & 16 deletions test/unit/canvas_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,9 @@
c._onDragLeave,
c._onDrop,
];
// initialize canvas more than once
c.initialize();
c.initialize();
// _initEventListeners canvas more than once
c._initEventListeners();
c._initEventListeners();
var eventsArray2 = [
c._onMouseDown,
c._onMouseMove,
Expand All @@ -510,7 +510,7 @@
c._onDragLeave,
c._onDrop,
];
assert.deepEqual(eventsArray, eventsArray2, 'after first initialize, functions do not change.');
assert.deepEqual(eventsArray, eventsArray2, 'after first _initEventListeners, functions do not change.');
});

['DragEnter', 'DragLeave', 'DragOver', 'Drop'].forEach(function(eventType) {
Expand All @@ -522,9 +522,9 @@
c[funcName] = function() {
counter++;
};
// initialize canvas more than once
c.initialize(c.lowerCanvasEl);
c.initialize(c.lowerCanvasEl);
// _initEventListeners canvas more than once
c._initEventListeners(c.lowerCanvasEl);
c._initEventListeners(c.lowerCanvasEl);
var event = fabric.document.createEvent('HTMLEvents');
event.initEvent(eventName, true, true);
c.upperCanvasEl.dispatchEvent(event);
Expand Down Expand Up @@ -769,9 +769,9 @@
c[funcName] = function() {
counter++;
};
// initialize canvas more than once
c.initialize(c.lowerCanvasEl);
c.initialize(c.lowerCanvasEl);
// _initEventListeners canvas more than once
c._initEventListeners(c.lowerCanvasEl);
c._initEventListeners(c.lowerCanvasEl);
var event = fabric.document.createEvent('MouseEvent');
event.initEvent(eventName, true, true);
c.upperCanvasEl.dispatchEvent(event);
Expand All @@ -787,9 +787,9 @@
counter++;
};
var c = new fabric.Canvas();
// initialize canvas more than once
c.initialize(c.lowerCanvasEl);
c.initialize(c.lowerCanvasEl);
// _initEventListeners canvas more than once
c._initEventListeners(c.lowerCanvasEl);
c._initEventListeners(c.lowerCanvasEl);

// a mouse down is necessary to register mouse up.
var _event = fabric.document.createEvent('MouseEvent');
Expand Down Expand Up @@ -857,9 +857,9 @@
counter++;
};
var c = new fabric.Canvas();
// initialize canvas more than once
c.initialize(c.lowerCanvasEl);
c.initialize(c.lowerCanvasEl);
// _initEventListeners canvas more than once
c._initEventListeners(c.lowerCanvasEl);
c._initEventListeners(c.lowerCanvasEl);
var event = fabric.document.createEvent('UIEvents');
event.initUIEvent('resize', true, false, fabric.window, 0);
fabric.window.dispatchEvent(event);
Expand Down
9 changes: 9 additions & 0 deletions test/unit/canvas_static.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,12 @@
}
});

QUnit.test('prevent multiple canvas initialization', function (assert) {
var canvas = new fabric.StaticCanvas();
assert.ok(canvas.lowerCanvasEl);
assert.throws(() => new fabric.StaticCanvas(canvas.lowerCanvasEl));
});

QUnit.test('initialProperties', function(assert) {
var canvas = new fabric.StaticCanvas();
assert.ok('backgroundColor' in canvas);
Expand Down Expand Up @@ -1555,9 +1561,12 @@
var canvas2 = new fabric.StaticCanvas(null, { renderOnAddRemove: false });
assert.ok(typeof canvas2.dispose === 'function');
canvas2.add(makeRect(), makeRect(), makeRect());
var lowerCanvas = canvas2.lowerCanvasEl;
assert.equal(lowerCanvas.getAttribute('data-fabric'), 'main', 'lowerCanvasEl should be marked by fabric');
canvas2.dispose();
assert.equal(canvas2.getObjects().length, 0, 'dispose should clear canvas');
assert.equal(canvas2.lowerCanvasEl, null, 'dispose should clear lowerCanvasEl');
assert.equal(lowerCanvas.hasAttribute('data-fabric'), false, 'dispose should clear lowerCanvasEl data-fabric attr');
assert.equal(canvas2.contextContainer, null, 'dispose should clear contextContainer');
});

Expand Down

0 comments on commit ec8d379

Please sign in to comment.