Merge pull request #424 from SVG-Edit/fix-undo-bug

Fix issue #423
master
JFH 2020-07-13 10:35:08 +02:00 committed by GitHub
commit b5e43c1e47
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 179 additions and 245 deletions

View File

@ -0,0 +1,33 @@
import {
visitAndApproveStorage
} from '../../../support/ui-test-helper.js';
// See https://github.com/SVG-Edit/svgedit/issues/423
describe('Fix issue 423', function () {
beforeEach(() => {
visitAndApproveStorage();
});
it('should not throw when undoing the move', function () {
cy.get('#tool_source').click();
cy.get('#svg_source_textarea')
.type('{selectall}')
.type(`<svg width="300" height="300" xmlns="http://www.w3.org/2000/svg" xmlns:svg="http://www.w3.org/2000/svg">
<g class="layer">
<title>Layer 1</title>
<g class="layer" id="svg_1">
<clipPath id="svg_2">
<rect height="150" id="svg_3" width="50" x="50" y="50"/>
</clipPath>
<rect clip-path="url(#svg_2)" fill="#0033b5" height="174.9" id="TANK1" width="78" x="77.5" y="29"/>
</g>
</g>
</svg>`, {parseSpecialCharSequences: false});
cy.get('#tool_source_save').click();
cy.get('#TANK1')
.trigger('mousedown', {force: true})
.trigger('mousemove', 50, 0, {force: true})
.trigger('mouseup', {force: true});
cy.get('#tool_undo').click();
});
});

View File

@ -18,8 +18,6 @@ export const HistoryEventTypes = {
AFTER_UNAPPLY: 'after_unapply'
};
// const removedElements = {};
/**
* Base class for commands.
*/
@ -30,6 +28,42 @@ class Command {
getText () {
return this.text;
}
/**
* @param {module:history.HistoryEventHandler} handler
* @param {callback} applyFunction
* @returns {void}
*/
apply (handler, applyFunction) {
handler && handler.handleHistoryEvent(HistoryEventTypes.BEFORE_APPLY, this);
applyFunction(handler);
handler && handler.handleHistoryEvent(HistoryEventTypes.AFTER_APPLY, this);
}
/**
* @param {module:history.HistoryEventHandler} handler
* @param {callback} unapplyFunction
* @returns {void}
*/
unapply (handler, unapplyFunction) {
handler && handler.handleHistoryEvent(HistoryEventTypes.BEFORE_UNAPPLY, this);
unapplyFunction();
handler && handler.handleHistoryEvent(HistoryEventTypes.AFTER_UNAPPLY, this);
}
/**
* @returns {Element[]} Array with element associated with this command
* This function needs to be surcharged if multiple elements are returned.
*/
elements () {
return [this.elem];
}
/**
* @returns {string} String with element associated with this command
*/
type () {
return this.constructor.name;
}
}
// Todo: Figure out why the interface members aren't showing
@ -71,11 +105,6 @@ class Command {
* @function module:history.HistoryCommand.type
* @returns {string}
*/
/**
* Gives the type.
* @function module:history.HistoryCommand#type
* @returns {string}
*/
/**
* @event module:history~Command#event:history
@ -116,12 +145,6 @@ export class MoveElementCommand extends Command {
this.newNextSibling = elem.nextSibling;
this.newParent = elem.parentNode;
}
/**
* @returns {"svgedit.history.MoveElementCommand"}
*/
type () { // eslint-disable-line class-methods-use-this
return 'svgedit.history.MoveElementCommand';
}
/**
* Re-positions the element.
@ -130,16 +153,9 @@ export class MoveElementCommand extends Command {
* @returns {void}
*/
apply (handler) {
// TODO(codedread): Refactor this common event code into a base HistoryCommand class.
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_APPLY, this);
}
super.apply(handler, () => {
this.elem = this.newParent.insertBefore(this.elem, this.newNextSibling);
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_APPLY, this);
}
});
}
/**
@ -149,26 +165,12 @@ export class MoveElementCommand extends Command {
* @returns {void}
*/
unapply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_UNAPPLY, this);
}
super.unapply(handler, () => {
this.elem = this.oldParent.insertBefore(this.elem, this.oldNextSibling);
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_UNAPPLY, this);
});
}
}
/**
* @returns {Element[]} Array with element associated with this command
*/
elements () {
return [this.elem];
}
}
MoveElementCommand.type = MoveElementCommand.prototype.type;
/**
* History command for an element that was added to the DOM.
* @implements {module:history.HistoryCommand}
@ -186,13 +188,6 @@ export class InsertElementCommand extends Command {
this.nextSibling = this.elem.nextSibling;
}
/**
* @returns {"svgedit.history.InsertElementCommand"}
*/
type () { // eslint-disable-line class-methods-use-this
return 'svgedit.history.InsertElementCommand';
}
/**
* Re-inserts the new element.
* @param {module:history.HistoryEventHandler} handler
@ -200,15 +195,9 @@ export class InsertElementCommand extends Command {
* @returns {void}
*/
apply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_APPLY, this);
}
super.apply(handler, () => {
this.elem = this.parent.insertBefore(this.elem, this.nextSibling);
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_APPLY, this);
}
});
}
/**
@ -218,27 +207,13 @@ export class InsertElementCommand extends Command {
* @returns {void}
*/
unapply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_UNAPPLY, this);
}
super.unapply(handler, () => {
this.parent = this.elem.parentNode;
this.elem.remove();
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_UNAPPLY, this);
});
}
}
/**
* @returns {Element[]} Array with element associated with this command
*/
elements () {
return [this.elem];
}
}
InsertElementCommand.type = InsertElementCommand.prototype.type;
/**
* History command for an element removed from the DOM.
* @implements {module:history.HistoryCommand}
@ -260,12 +235,6 @@ export class RemoveElementCommand extends Command {
// special hack for webkit: remove this element's entry in the svgTransformLists map
removeElementFromListMap(elem);
}
/**
* @returns {"svgedit.history.RemoveElementCommand"}
*/
type () { // eslint-disable-line class-methods-use-this
return 'svgedit.history.RemoveElementCommand';
}
/**
* Re-removes the new element.
@ -274,17 +243,11 @@ export class RemoveElementCommand extends Command {
* @returns {void}
*/
apply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_APPLY, this);
}
super.apply(handler, () => {
removeElementFromListMap(this.elem);
this.parent = this.elem.parentNode;
this.elem.remove();
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_APPLY, this);
}
});
}
/**
@ -294,10 +257,7 @@ export class RemoveElementCommand extends Command {
* @returns {void}
*/
unapply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_UNAPPLY, this);
}
super.unapply(handler, () => {
removeElementFromListMap(this.elem);
if (isNullish(this.nextSibling)) {
if (window.console) {
@ -305,21 +265,10 @@ export class RemoveElementCommand extends Command {
}
}
this.parent.insertBefore(this.elem, this.nextSibling); // Don't use `before` or `prepend` as `this.nextSibling` may be `null`
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_UNAPPLY, this);
});
}
}
/**
* @returns {Element[]} Array with element associated with this command
*/
elements () {
return [this.elem];
}
}
RemoveElementCommand.type = RemoveElementCommand.prototype.type;
/**
* @typedef {"#text"|"#href"|string} module:history.CommandAttributeName
*/
@ -354,24 +303,15 @@ export class ChangeElementCommand extends Command {
}
}
}
/**
* @returns {"svgedit.history.ChangeElementCommand"}
*/
type () { // eslint-disable-line class-methods-use-this
return 'svgedit.history.ChangeElementCommand';
}
/**
* Performs the stored change action.
* @param {module:history.HistoryEventHandler} handler
* @fires module:history~Command#event:history
* @returns {true}
* @returns {void}
*/
apply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_APPLY, this);
}
super.apply(handler, () => {
let bChangedTransform = false;
Object.entries(this.newValues).forEach(([attr, value]) => {
if (value) {
@ -397,33 +337,25 @@ export class ChangeElementCommand extends Command {
const angle = getRotationAngle(this.elem);
if (angle) {
const bbox = this.elem.getBBox();
const cx = bbox.x + bbox.width / 2,
cy = bbox.y + bbox.height / 2;
const cx = bbox.x + bbox.width / 2;
const cy = bbox.y + bbox.height / 2;
const rotate = ['rotate(', angle, ' ', cx, ',', cy, ')'].join('');
if (rotate !== this.elem.getAttribute('transform')) {
this.elem.setAttribute('transform', rotate);
}
}
}
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_APPLY, this);
}
return true;
});
}
/**
* Reverses the stored change action.
* @param {module:history.HistoryEventHandler} handler
* @fires module:history~Command#event:history
* @returns {true}
* @returns {void}
*/
unapply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_UNAPPLY, this);
}
super.unapply(handler, () => {
let bChangedTransform = false;
Object.entries(this.oldValues).forEach(([attr, value]) => {
if (value) {
@ -454,25 +386,11 @@ export class ChangeElementCommand extends Command {
}
}
}
// Remove transformlist to prevent confusion that causes bugs like 575.
removeElementFromListMap(this.elem);
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_UNAPPLY, this);
}
return true;
}
/**
* @returns {Element[]} Array with element associated with this command
*/
elements () {
return [this.elem];
});
}
}
ChangeElementCommand.type = ChangeElementCommand.prototype.type;
// TODO: create a 'typing' command object that tracks changes in text
// if a new Typing command is created and the top command on the stack is also a Typing
@ -492,13 +410,6 @@ export class BatchCommand extends Command {
this.stack = [];
}
/**
* @returns {"svgedit.history.BatchCommand"}
*/
type () { // eslint-disable-line class-methods-use-this
return 'svgedit.history.BatchCommand';
}
/**
* Runs "apply" on all subcommands.
* @param {module:history.HistoryEventHandler} handler
@ -506,18 +417,12 @@ export class BatchCommand extends Command {
* @returns {void}
*/
apply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_APPLY, this);
}
const len = this.stack.length;
for (let i = 0; i < len; ++i) {
this.stack[i].apply(handler);
}
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_APPLY, this);
}
super.apply(handler, () => {
this.stack.forEach((stackItem) => {
console.assert(stackItem, 'stack item should not be null');
stackItem && stackItem.apply(handler);
});
});
}
/**
@ -527,17 +432,12 @@ export class BatchCommand extends Command {
* @returns {void}
*/
unapply (handler) {
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.BEFORE_UNAPPLY, this);
}
for (let i = this.stack.length - 1; i >= 0; i--) {
this.stack[i].unapply(handler);
}
if (handler) {
handler.handleHistoryEvent(HistoryEventTypes.AFTER_UNAPPLY, this);
}
super.unapply(handler, () => {
this.stack.forEach((stackItem) => {
console.assert(stackItem, 'stack item should not be null');
stackItem && stackItem.unapply(handler);
});
});
}
/**
@ -548,6 +448,7 @@ export class BatchCommand extends Command {
const elems = [];
let cmd = this.stack.length;
while (cmd--) {
if (!this.stack[cmd]) continue;
const thisElems = this.stack[cmd].elements();
let elem = thisElems.length;
while (elem--) {
@ -563,6 +464,7 @@ export class BatchCommand extends Command {
* @returns {void}
*/
addSubCommand (cmd) {
console.assert(cmd !== null, 'cmd should not be null');
this.stack.push(cmd);
}
@ -573,7 +475,6 @@ export class BatchCommand extends Command {
return !this.stack.length;
}
}
BatchCommand.type = BatchCommand.prototype.type;
/**
*

View File

@ -505,17 +505,17 @@ const undoMgr = canvas.undoMgr = new UndoManager({
call('changed', elems);
const cmdType = cmd.type();
const isApply = (eventType === EventTypes.AFTER_APPLY);
if (cmdType === MoveElementCommand.type()) {
if (cmdType === 'MoveElementCommand') {
const parent = isApply ? cmd.newParent : cmd.oldParent;
if (parent === svgcontent) {
draw.identifyLayers();
}
} else if (cmdType === InsertElementCommand.type() ||
cmdType === RemoveElementCommand.type()) {
} else if (cmdType === 'InsertElementCommand' ||
cmdType === 'RemoveElementCommand') {
if (cmd.parent === svgcontent) {
draw.identifyLayers();
}
if (cmdType === InsertElementCommand.type()) {
if (cmdType === 'InsertElementCommand') {
if (isApply) { restoreRefElems(cmd.elem); }
} else if (!isApply) {
restoreRefElems(cmd.elem);
@ -523,7 +523,7 @@ const undoMgr = canvas.undoMgr = new UndoManager({
if (cmd.elem && cmd.elem.tagName === 'use') {
setUseData(cmd.elem);
}
} else if (cmdType === ChangeElementCommand.type()) {
} else if (cmdType === 'ChangeElementCommand') {
// if we are changing layer names, re-identify all layers
if (cmd.elem.tagName === 'title' &&
cmd.elem.parentNode.parentNode === svgcontent

View File

@ -28,9 +28,9 @@ const $ = jQueryPluginSVG(jQuery);
const KEYSTR = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=';
// Much faster than running getBBox() every time
const visElems = 'a,circle,ellipse,foreignObject,g,image,line,path,polygon,polyline,rect,svg,text,tspan,use';
const visElems = 'a,circle,ellipse,foreignObject,g,image,line,path,polygon,polyline,rect,svg,text,tspan,use,clipPath';
const visElemsArr = visElems.split(',');
// const hidElems = 'clipPath,defs,desc,feGaussianBlur,filter,linearGradient,marker,mask,metadata,pattern,radialGradient,stop,switch,symbol,title,textPath';
// const hidElems = 'defs,desc,feGaussianBlur,filter,linearGradient,marker,mask,metadata,pattern,radialGradient,stop,switch,symbol,title,textPath';
let editorContext_ = null;
let domdoc_ = null;