Undo removes multiple execution listeners at a time

[pinussilvestrus]

Describe the bug

When I add multiple execution listeners to a C7 BPMN diagram and press CMD+Z to undo, it removes multiple (not all) listeners and not only the last one added.

Steps to reproduce

1. Select a service task
2. Add multiple execution listeners
3. Undo changes (e.g. via CTRL/CMD+Z)
4. See multiple listeners were removed, not only the last one added

Expected behavior

Only the last added listener should be removed in this scenario.

Note that it works as expected for other list groups (e.g. Inputs and Outputs).

Environment

• OS: MacOS 11.6.5
• Camunda Modeler Version: 5.1.0-rc.1, 5.0.0
• Execution Platform: Camunda Platform 7.15
• Installed plug-ins: Cloud Connect Modeler Plugin

Additional context

No response

[pinussilvestrus]

Note that redo also brings back all removed listeners. It seems like the command stack is combining multiple add operations in this case.

[pinussilvestrus]

Moving this to the backlog and adding the spring cleaning label as it is not a blocker.

[smbea]

I can reproduce this in the Modeler but not in bpmn-js-properties-panel or camunda-bpmn-js 🤔

[pinussilvestrus]

I can reproduce this in the Modeler but not in bpmn-js-properties-panel or camunda-bpmn-js 🤔

That could be an issue with the modeling behaviors then: https://github.com/camunda/camunda-bpmn-js-behaviors

[smbea]

But if that were the case it should be reflected on camunda-bpmn-js right?

[pinussilvestrus]

But if that were the case it should be reflected on camunda-bpmn-js right?

Ah sure thing, sorry 🙈

[philippfromme]

I'll try my luck. 🤡

[philippfromme]

tl;dr

Properties panel keyboard events aren't stopped from being propagated and therefore are handled twice. Only thanks to a quirk does this not result in undo being triggered twice in most cases.

Root Cause Analysis

• there are two places in the Camunda Modeler where we trigger undo and redo
  • KeyboardBindings - triggers undo and redo if edit menu contains entries and they're enabled
  • PropertiesPanelKeyboardBindings - triggers command stack undo and redo instead of native when properties panel container is focused

The expected behavior when working with the properties panel is:

• the properties panel is focused e.g. an input is focused
• when CMD + Z is pressed undo is triggered by PropertiesPanelKeyboardBindings
• no other keyboard listeners are triggered

The actual behavior when working with the properties panel is:

• the properties panel is focused e.g. an input is focused
• when CMD + Z is pressed undo is triggered by PropertiesPanelKeyboardBindings
• the event is propagated and undo is triggered again by KeyboardBindings

The reason why this behavior can only be observed when adding execution listeners is that when adding an execution listener no input of the execution listener will be focused.

Compare that to adding inputs.

When focusing a properties panel input the following happens:

• edit menu is updated
• since an input is focused native undo and redo entries are used (cf. https://github.com/camunda/camunda-modeler/blob/v5.2.0/client/src/app/tabs/bpmn/BpmnEditor.js#L397)
• when pressing CMD + Z KeyboardBindings will not trigger command stack undo since native undo and redo entries are used

Since adding an execution listener doesn't focus an input the edit menu isn't updated with the native undo and redo entries. So it is only thanks to the edit menu update that in most cases undo isn't triggered twice.

What's missing in PropertiesPanelKeyboardBindings is to stop the event from being propagated. This was probably intended but forgotten to implement. preventDefault is called but stopPropagation isn't (cf. https://github.com/camunda/camunda-modeler/blob/develop/client/src/app/tabs/bpmn/modeler/features/properties-panel-keyboard-bindings/PropertiesPanelKeyoardBindings.js#L59).