search

bpmn-io / bpmn-js

A BPMN 2.0 rendering toolkit and web modeler.
https://bpmn.io/toolkit/bpmn-js/
Other
8.55k stars 1.32k forks source link

HTML validation errors #2179

Closed boris-petrov closed 4 months ago

boris-petrov commented 4 months ago

Describe the Bug

Using the newest version, I get a lot of errors from my HTML validator like Duplicate ID “marker-dgx997t15d1210s2srnctq0ez”. Indeed, in the HTML I see two of that ID (and all the rest that the validator complains about). I'm not sure which version introduced that but it's recent - in the past week or two. I guess the problem might be here somewhere.

Steps to Reproduce

Just render any diagram in the modeler.

Expected Behavior

No HTML-validation errors.

Environment

Depends on https://github.com/bpmn-io/diagram-js/pull/909.

nikku commented 4 months ago

@marstamm Could be related to https://github.com/bpmn-io/bpmn-js/pull/2173?

nikku commented 4 months ago

@boris-petrov Please share additional details on your setup:

nikku commented 4 months ago

I can verify the report during dragging; if we clone elements with an ID we must change the ID or duplicates will occur:

image

In my tests there is one original element with a certain ID and three .djs-dragger elements (with duplicate IDs).

marstamm commented 4 months ago

Yes, this is related to the latest changes. We have 2 issues here:

  1. For preview, the comple SVG is copied, including the marker with the ID. We only add a cloned marker, but do not remove the copied one. This issue is new. This needs to be fixed in preview support.
  2. For drag-preview, we add the sequence flow multiple times. We add a connection from start shape, end shape and the connection itself. This is reproduceable on earlier version as well. Needs to be fixed in Move Preview.
nikku commented 4 months ago

Closed via https://github.com/bpmn-io/bpmn-js/issues/2181.

boris-petrov commented 4 months ago

Hi, thanks for the quick fix and sorry I didn't provide enough detail. I tried updating to diagram-js@14.7.1 but I still get the same errors. I've attached the HTML (as TXT as GitHub doesn't allow HTML) that results in the error. The duplicate ID is marker-6cnzxp45vcldqdsc7bp8xadlw. You can see it twice in the HTML. My code is as follows:

  <div class="canvas"></div>
  <div class="properties"></div>
    this.modeler = new BpmnModeler({
      container: $('.canvas').get(0)!,
      keyboard: {
        bindTo: document,
      },
      propertiesPanel: {
        parent: $('.properties').get(0),
      },
      bpmnRenderer: {
        defaultFillColor: 'var(--bs-body-bg)',
        defaultStrokeColor: 'var(--bs-body-color)',
      },
    });

Let me know if that helps or you need more information?

bpmn-bug.txt

marstamm commented 4 months ago

The duplicated markers come from the minimap. Seems like it copies the SVG and only strips the root defs

marstamm commented 4 months ago

Correction, we simply clone everything in the djs-visual when we replicate the elements in the minimap: https://github.com/bpmn-io/diagram-js-minimap/blob/156bcaa2d64013a13f4c601c013bb466ab124eed/lib/Minimap.js#L741-L746

we need to strip or change the markers there. Will look into this tomorrow

marstamm commented 4 months ago

Given the issues we are facing in multiple sections of our code, I'll also evaluate if there are other solutions than rendering as part of djs-visual container

marstamm commented 4 months ago

Discussed with @nikku today: