Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML validation errors #2179

Closed
boris-petrov opened this issue May 28, 2024 · 10 comments
Closed

HTML validation errors #2179

boris-petrov opened this issue May 28, 2024 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@boris-petrov
Copy link
Contributor

boris-petrov commented May 28, 2024

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

  • Browser: Any
  • OS: Linux
  • Library version: 17.8.0

Depends on bpmn-io/diagram-js#909.

@boris-petrov boris-petrov added the bug Something isn't working label May 28, 2024
@nikku nikku added the ready Ready to be worked on label May 28, 2024
@nikku
Copy link
Member

nikku commented May 28, 2024

@marstamm Could be related to #2173?

@nikku
Copy link
Member

nikku commented May 29, 2024

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

  • How do you embed bpmn-js
  • How do you validate the HTML to find these issues
  • If possible provide us with an export of the HTML for inspection

@nikku
Copy link
Member

nikku commented May 29, 2024

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
Copy link
Contributor

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.

marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
nikku pushed a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
nikku pushed a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
marstamm added a commit to bpmn-io/diagram-js that referenced this issue May 29, 2024
@nikku nikku added the fixed upstream Requires integration of upstream change label May 29, 2024 — with bpmn-io-tasks
@nikku nikku removed the ready Ready to be worked on label May 29, 2024
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed fixed upstream Requires integration of upstream change labels May 29, 2024
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels May 29, 2024
@nikku
Copy link
Member

nikku commented May 29, 2024

Closed via #2181.

@nikku nikku closed this as completed May 29, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 29, 2024
@boris-petrov
Copy link
Contributor Author

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
Copy link
Contributor

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

@marstamm
Copy link
Contributor

marstamm commented May 29, 2024

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
Copy link
Contributor

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
Copy link
Contributor

Discussed with @nikku today:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

3 participants