Group name/label with whitespaces only crashes modeler

[torge-hmn]

Describe the Bug

If the name/label of a group (= value of the referenced categoryValue) is initially set to a whitespace only string, the modeler crashes (= no useful interaction is possible anymore - can't move elements and creation of new elements is scuffed).

This behavior does not occur, when the group already had a valid name/label before. My strong assumption is that the implicit creation of the categoryValue is not taking place in case of a whitespace only value.

Steps to Reproduce

1. Create new model on https://demo.bpmn.io/
2. Add a new group
3. Double click group for direct editing of the label
4. Enter a whitespace only string
5. Leave direct editing by clicking anywhere in the model

Expected Behavior

No crash. Either the group should get no name/label at all or the whitespace string should be used.

Environment

• Browser: any
• OS: any
• Library version: 16+ (only tested in 16 and 17)

---

Crash report, visible in the console:

Uncaught TypeError: Cannot set properties of undefined (setting 'value')
    at LabelUtil.js:254:36
    at r (UpdateLabelHandler.js:54:5)
    at gy.execute (UpdateLabelHandler.js:89:12)
    at CommandStack.js:452:31
    at Kv._atomicDo (CommandStack.js:416:5)
    at Kv._internalExecute (CommandStack.js:445:8)
    at Kv.execute (CommandStack.js:186:8)
    at by.updateLabel (Modeling.js:106:22)
    at XA.update (LabelEditingProvider.js:495:18)
    at jb.complete (DirectEditing.js:105:21)
    at jb._handleKey (DirectEditing.js:140:17)
    at HTMLDivElement.o (helpers.ts:98:17)

[nikku]

Thanks for your report, I can reproduce your issue.

Do you want to contribute a fix?

[torge-hmn]

I could take a look into it when I have some spare time - do you have a suggestion where I should start and where a fix would fit best?

[nikku]

@torge-hmn I had a quick look myself and attached the crash report visible in the developer console, reachable via F12 in many modern browsers.

The trace allows us to deduce where the error is coming from, create a test case and address it.

[torge-hmn]

@torge-hmn I had a quick look myself and attached the crash report visible in the developer console, reachable via F12 in many modern browsers.

The trace allows us to deduce where the error is coming from, create a test case and address it.

Thanks for your reply and attaching the stack trace - I already looked at the stack trace but forgot to attach it to the issue.

I had a few minutes trying to examine the cause of the error and started implementing a reproducer test.

I'm not quite sure but maybe this check leads to the crash: During preExecute of the label update a new label is not created in case of an empty value. This might lead to the category and categoryValue not being created before the label is then updated during execution. This update fails as it expects both elements to exist.

I didn't have time to finish my reproducer tests and try fixing the issue, but do you think I am right about the cause or at least on track in the right direction?

[torge-hmn]

I've now created two tests (second one is the reproducer) calling updateLabel on groups without an existing categoryValueRef in this commit.

As I'm not sure how you would like to fix the issue, I've provided a basic fix that at least fixes the reproducer test and prevents a modeller crash in this commit.

I thought about some approaches for a better fix, but I'm not sure what fits the library best. Here are my ideas in order of my personal preference (which does not really matter 😄):

1. Extend the current example fix to take the text and categoryValueRef into account: If no categoryValueRef is present and the text is empty this should not trigger an update and setting the value could just be skipped. In case of the text being not empty, the update would take place like before, as we would expect the categoryValueRef to be created in preExecute. Additionally, if the text is empty and a categoryValueRef is present, the value would still be set to the empty text to allow removing a label (but still keeping the category like before): reference
2. Don't trigger a label update, if old and new label are empty in label update execution: reference
3. Add a special case for groups and always create a category and categoryValue in preExecute even if the label is empty: reference
4. Implicitly create missing category and categoryValue inside of setLabel if they are missing: reference

Maybe one of those ideas fits your thoughts and if not, please tell me how you would like the issue to be fixed. I would adjust my fix accordingly and create a PR - thanks!

[nikku]

Amazing stuff. Do you want to open a PR with your changes? We can take it from there.

I.e. we are fine with a draft PR only, to continue the discussion, and evolve the solution from there.

[torge-hmn]

Thanks for creating the pr @abdul99ahad! I didn't have time until now and just saw you already opened a pr.

[nikku]

@torge-hmn We should have given you a heads-up on work happening on our end 🙈. Yes, the PR to address this issue is up, and we hope to get it merged soon.

[torge-hmn]

@torge-hmn We should have given you a heads-up on work happening on our end 🙈. Yes, the PR to address this issue is up, and we hope to get it merged soon.

Oh, that's not a problem at all. I'm happy you're addressing the issue that quickly 😄