From e5a3973107932dcfd7a30c19d75e5fcaf83745e1 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Tue, 10 Apr 2018 09:33:11 +0200 Subject: [PATCH] fix(replace-preview): escape element ids in CSS selectors --- .wiredeps | 3 + CHANGELOG.md | 4 + .../replace-preview/BpmnReplacePreview.js | 6 +- package.json | 3 +- .../replace-preview/BpmnReplacePreview.bpmn | 47 +++++++++ .../replace-preview/BpmnReplacePreviewSpec.js | 98 ++++++++++--------- 6 files changed, 114 insertions(+), 47 deletions(-) create mode 100644 test/spec/features/replace-preview/BpmnReplacePreview.bpmn diff --git a/.wiredeps b/.wiredeps index e8f9e898..9e17a9eb 100644 --- a/.wiredeps +++ b/.wiredeps @@ -29,6 +29,9 @@ "component-event": { "version": "0.1.4" }, + "css.escape": { + "version": "1.5.1" + }, "delegate-events": { "version": "1.1.1", "requires": { diff --git a/CHANGELOG.md b/CHANGELOG.md index 8aabbd40..8d5796d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to [bpmn-js](https://github.com/bpmn-io/bpmn-js) are documen ___Note:__ Yet to be released changes appear here._ +## 1.1.1 + +* `FIX`: escape `data-element-id` in CSS selectors + ## 1.1.0 * `FEAT`: show gateway icon on context pad without marker ([`15dfab6b`](https://github.com/bpmn-io/bpmn-js/commit/15dfab6b5b12dd184acf070f2ab3ad205d1b245c)) diff --git a/lib/features/replace-preview/BpmnReplacePreview.js b/lib/features/replace-preview/BpmnReplacePreview.js index f6416fbe..8ae9cb31 100644 --- a/lib/features/replace-preview/BpmnReplacePreview.js +++ b/lib/features/replace-preview/BpmnReplacePreview.js @@ -2,6 +2,8 @@ import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor'; import inherits from 'inherits'; +import cssEscape from 'css.escape'; + import { assign, forEach @@ -56,7 +58,7 @@ export default function BpmnReplacePreview( canvas.addShape(tempShape, element.parent); // select the original SVG element related to the element and hide it - var gfx = domQuery('[data-element-id=' + element.id + ']', context.dragGroup); + var gfx = domQuery('[data-element-id="' + cssEscape(element.id) + '"]', context.dragGroup); if (gfx) { svgAttr(gfx, { display: 'none' }); @@ -82,7 +84,7 @@ export default function BpmnReplacePreview( forEach(visualReplacements, function(dragger, id) { - var originalGfx = domQuery('[data-element-id=' + id + ']', context.dragGroup); + var originalGfx = domQuery('[data-element-id="' + cssEscape(id) + '"]', context.dragGroup); if (originalGfx) { svgAttr(originalGfx, { display: 'inline' }); diff --git a/package.json b/package.json index fab69fae..94ef8114 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,8 @@ "dependencies": { "bpmn-font": "^0.8.0", "bpmn-moddle": "^5.1.0", - "diagram-js": "^1.1.0", + "css.escape": "^1.5.1", + "diagram-js": "^1.2.2", "diagram-js-direct-editing": "^1.2.0", "ids": "^0.2.0", "inherits": "^2.0.1", diff --git a/test/spec/features/replace-preview/BpmnReplacePreview.bpmn b/test/spec/features/replace-preview/BpmnReplacePreview.bpmn new file mode 100644 index 00000000..007b63b5 --- /dev/null +++ b/test/spec/features/replace-preview/BpmnReplacePreview.bpmn @@ -0,0 +1,47 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/spec/features/replace-preview/BpmnReplacePreviewSpec.js b/test/spec/features/replace-preview/BpmnReplacePreviewSpec.js index 4236c2b6..1391ef14 100644 --- a/test/spec/features/replace-preview/BpmnReplacePreviewSpec.js +++ b/test/spec/features/replace-preview/BpmnReplacePreviewSpec.js @@ -25,9 +25,9 @@ import { describe('features/replace-preview', function() { - var diagramXML = require('../../../fixtures/bpmn/event-sub-processes.bpmn'); + var diagramXML = require('./BpmnReplacePreview.bpmn'); - var startEvent_1, + var startEvent1, rootElement; var getGfx, @@ -44,7 +44,7 @@ describe('features/replace-preview', function() { beforeEach(inject(function(canvas, elementRegistry, elementFactory, move, dragging) { - startEvent_1 = elementRegistry.get('StartEvent_1'); + startEvent1 = elementRegistry.get('StartEvent.1'); rootElement = canvas.getRootElement(); /** @@ -69,7 +69,10 @@ describe('features/replace-preview', function() { }; moveShape = function(shape, target, position) { - var startPosition = { x: shape.x + 10 + (shape.width / 2), y: shape.y + 30 + (shape.height / 2) }; + var startPosition = { + x: shape.x + 10 + (shape.width / 2), + y: shape.y + 30 + (shape.height / 2) + }; move.start(canvasEvent(startPosition), shape); @@ -84,10 +87,10 @@ describe('features/replace-preview', function() { })); - it('should replace visuals at the same position as the replaced visual', inject(function(dragging) { + it('should draw replaced visuals at correct position', inject(function(dragging) { // when - moveShape(startEvent_1, rootElement, { x: 280, y: 120 }); + moveShape(startEvent1, rootElement, { x: 280, y: 120 }); // then var dragGroup = dragging.context().data.context.dragGroup; @@ -101,40 +104,42 @@ describe('features/replace-preview', function() { it('should add dragger to context.visualReplacements once', inject(function(dragging) { // when - moveShape(startEvent_1, rootElement, { x: 275, y: 120 }); - moveShape(startEvent_1, rootElement, { x: 280, y: 120 }); - moveShape(startEvent_1, rootElement, { x: 285, y: 120 }); + moveShape(startEvent1, rootElement, { x: 275, y: 120 }); + moveShape(startEvent1, rootElement, { x: 280, y: 120 }); + moveShape(startEvent1, rootElement, { x: 285, y: 120 }); // then var visualReplacements = dragging.context().data.context.visualReplacements; - expect(visualReplacements[startEvent_1.id]).to.exist; + expect(visualReplacements[startEvent1.id]).to.exist; expect(Object.keys(visualReplacements).length).to.equal(1); })); - it('should remove dragger from context.visualReplacements', inject(function(elementRegistry, dragging) { + it('should remove dragger from context.visualReplacements', inject( + function(elementRegistry, dragging) { - // given - var subProcess_2 = elementRegistry.get('SubProcess_2'); + // given + var subProcess2 = elementRegistry.get('SubProcess_2'); - // when - moveShape(startEvent_1, rootElement, { x: 275, y: 120 }); - moveShape(startEvent_1, rootElement, { x: 280, y: 120 }); - moveShape(startEvent_1, subProcess_2, { x: 350, y: 120 }); + // when + moveShape(startEvent1, rootElement, { x: 275, y: 120 }); + moveShape(startEvent1, rootElement, { x: 280, y: 120 }); + moveShape(startEvent1, subProcess2, { x: 350, y: 120 }); - // then - var visualReplacements = dragging.context().data.context.visualReplacements; + // then + var visualReplacements = dragging.context().data.context.visualReplacements; - expect(visualReplacements).to.be.empty; - })); + expect(visualReplacements).to.be.empty; + } + )); it('should hide the replaced visual', inject(function(dragging) { // when - moveShape(startEvent_1, rootElement, { x: 280, y: 120 }); + moveShape(startEvent1, rootElement, { x: 280, y: 120 }); // then var dragGroup = dragging.context().data.context.dragGroup; @@ -147,10 +152,10 @@ describe('features/replace-preview', function() { inject(function(dragging, elementRegistry) { // given - var subProcess_1 = elementRegistry.get('SubProcess_1'); + var subProcess1 = elementRegistry.get('SubProcess.1'); // when - moveShape(startEvent_1, subProcess_1, { x: 210, y: 180 }); + moveShape(startEvent1, subProcess1, { x: 210, y: 180 }); var context = dragging.context().data.context; @@ -171,7 +176,7 @@ describe('features/replace-preview', function() { inject(function(dragging, elementRegistry) { // when - moveShape(startEvent_1, rootElement, { x: 280, y: 120 }); + moveShape(startEvent1, rootElement, { x: 280, y: 120 }); var context = dragging.context().data.context; @@ -188,10 +193,10 @@ describe('features/replace-preview', function() { inject(function(dragging, elementRegistry) { // given - var subProcess_2 = elementRegistry.get('SubProcess_2'); + var subProcess2 = elementRegistry.get('SubProcess_2'); // when - moveShape(startEvent_1, subProcess_2, { x: 350, y: 120 }); + moveShape(startEvent1, subProcess2, { x: 350, y: 120 }); var context = dragging.context().data.context; @@ -212,10 +217,10 @@ describe('features/replace-preview', function() { inject(function(dragging, elementRegistry) { // given - var subProcess_3 = elementRegistry.get('SubProcess_3'); + var subProcess3 = elementRegistry.get('SubProcess_3'); // when - moveShape(startEvent_1, subProcess_3, { x: 600, y: 120 }); + moveShape(startEvent1, subProcess3, { x: 600, y: 120 }); var context = dragging.context().data.context; @@ -232,13 +237,13 @@ describe('features/replace-preview', function() { inject(function(move, dragging, elementRegistry, selection) { // given - var startEvent_2 = elementRegistry.get('StartEvent_2'), - startEvent_3 = elementRegistry.get('StartEvent_3'); + var startEvent2 = elementRegistry.get('StartEvent.2'), + startEvent3 = elementRegistry.get('StartEvent.3'); // when - selection.select([ startEvent_1, startEvent_2, startEvent_3 ]); + selection.select([ startEvent1, startEvent2, startEvent3 ]); - moveShape(startEvent_1, rootElement, { x: 150, y: 250 }); + moveShape(startEvent1, rootElement, { x: 150, y: 250 }); var context = dragging.context().data.context; @@ -257,9 +262,9 @@ describe('features/replace-preview', function() { inject(function(move, dragging, elementRegistry, selection) { // given - var startEvent_2 = elementRegistry.get('StartEvent_2'), - startEvent_3 = elementRegistry.get('StartEvent_3'), - subProcess_2 = elementRegistry.get('SubProcess_2'); + var startEvent2 = elementRegistry.get('StartEvent.2'), + startEvent3 = elementRegistry.get('StartEvent.3'), + subProcess2 = elementRegistry.get('SubProcess_2'); var messageStartEventGfx = getGfx({ type: 'bpmn:StartEvent', @@ -276,9 +281,9 @@ describe('features/replace-preview', function() { var startEventGfx = getGfx({ type: 'bpmn:StartEvent' }); // when - selection.select([ startEvent_1, startEvent_2, startEvent_3 ]); + selection.select([ startEvent1, startEvent2, startEvent3 ]); - moveShape(startEvent_1, subProcess_2, { x: 350, y: 120 }); + moveShape(startEvent1, subProcess2, { x: 350, y: 120 }); var context = dragging.context().data.context; @@ -294,18 +299,23 @@ describe('features/replace-preview', function() { inject(function(move, dragging, elementRegistry, elementFactory, selection, modeling) { // given - var startEvent_1 = elementRegistry.get('StartEvent_1'), - subProcess_3 = elementRegistry.get('SubProcess_3'); + var startEvent1 = elementRegistry.get('StartEvent.1'), + subProcess3 = elementRegistry.get('SubProcess_3'); var intermediateEvent = elementFactory.createShape({ type: 'bpmn:IntermediateThrowEvent' }); - var boundaryEvent = modeling.createShape(intermediateEvent, { x: 550, y: 180 }, subProcess_3, true); + var boundaryEvent = modeling.createShape( + intermediateEvent, + { x: 550, y: 180 }, + subProcess3, + true + ); // when - selection.select([ startEvent_1 ]); + selection.select([ startEvent1 ]); - moveShape(boundaryEvent, subProcess_3, { x: 580, y: 210 }); - moveShape(boundaryEvent, subProcess_3, { x: 580, y: 180 }); + moveShape(boundaryEvent, subProcess3, { x: 580, y: 210 }); + moveShape(boundaryEvent, subProcess3, { x: 580, y: 180 }); // then // expect not to throw TypeError: Cannot read property 'oldElementId' of undefined