Namespaces get removed when exporting foreign diagram #1310

linus-amg · 2020-04-09T20:19:01Z

Describe the Bug

A diagram I exported from a different Modeler and afterwards imported into bpmn-js is not schema valid anymore as soon as I export it from bpmn-js.

This issue is coming from running tests for the upcoming bpmn miwg demo.

Fixtures
Advertise a job vacancy.original.edit.bpmn.zip

Steps to Reproduce

Have a look at the attached xml and see that xmlns:dc, xmlns:di and xmlns:bpmndi are part of <defintions>
Import attached XML into a bpmn-js instance
use the saveXML method of the bpmn-js instance to get the xml
see that the xmlns:dc, xmlns:di and xmlns:bpmndi attributes are not present in the definitions anymore

I tried to create a showcase: https://codesandbox.io/s/busy-dawn-eel38?file=/src/index.js just have a look at the console tab to see the different attributes before and after import+export.

Expected Behavior

I expected the 3 mentioned namespaces to still be inside definitions, so importing and exporting this model which was created in a different modeler is still schema-valid.

Impact
It's a blocker for the bpmn miwg demo because we cannot import models from CaseAgile and export them, to make the roundtrip.

Environment

OS: Linux, MacOS
Library version: 6.4.1
Also tried Camunda Modeler 3.7, 4 and Cawemo and results are the same.

The text was updated successfully, but these errors were encountered:

Related to bpmn-io/bpmn-js#1310

nikku · 2020-04-10T21:23:21Z

Added basic reproducible test cases in bpmn-moddle as well as moddle-xml.

We'll look into this.

Our tools should not ever export invalid XML, no matter how cryptic / special the input XML is.

falko · 2020-04-13T10:04:21Z

@nikku thank you for looking into this!

falko · 2020-04-13T10:22:28Z

When trying to root cause this, I made an observation:
The attached BPMN file uses elements from the BPMN semantic namespace without a namespace prefix and correctly declares that namespace using xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" on the definitions root element. So that looks fine.
In addition, it defines the prefix semantic for the same namespace through xmlns:semantic="http://www.omg.org/spec/BPMN/20100524/MODEL" but never uses it. Technically that's useless but not invalid. However, we start using that prefix when we export that file. While that's also not invalid, such behavior could lead to problems for people who work with branching and merging, e.g. in Git, because almost every line of the file is changed.

Removing the unused namespace semantic prefix makes the modeler keep the unprefixed elements. However, it does not fix the issue of the missing bpmndi, di, and dc namespace prefix declarations. So maybe this observation is even a separate issue.

nikku · 2020-04-13T12:34:31Z

I've already root caused these things, too.

See linked, focussed test cases attached.

Regarding the namespaces and the specific example, what namespace exports would you expect?

<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<definitions id="Definitions" xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" targetNamespace="http://bpmn.io">
  <BPMNDiagram id="BPMNDiagram_1" xmlns="http://www.omg.org/spec/BPMN/20100524/DI">
    <BPMNPlane bpmnElement="Definitions" xmlns="http://www.omg.org/spec/BPMN/20100524/DI" />
  </BPMNDiagram>
</definitions>

nikku · 2020-04-13T12:37:01Z

In addition, it defines the prefix semantic for the same namespace through xmlns:semantic="http://www.omg.org/spec/BPMN/20100524/MODEL" but never uses it. Technically that's useless but not invalid. However, we start using that prefix when we export that file. While that's also not invalid, such behavior could lead to problems for people who work with branching and merging, e.g. in Git, because almost every line of the file is changed.

As you mentioned, this is a different issue.

falko · 2020-04-13T12:52:33Z

Regarding the namespaces and the specific example, what namespace exports would you expect?

The namespaces that are used in the model. In particular, the namespace prefixes bpmndi, di, and dc were undeclared. Schema validation can be used to verify.

nikku · 2020-04-13T18:08:57Z

So the export would look as imported?

<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<definitions id="Definitions" xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" targetNamespace="http://bpmn.io">
  <BPMNDiagram id="BPMNDiagram_1" xmlns="http://www.omg.org/spec/BPMN/20100524/DI">
    <BPMNPlane bpmnElement="Definitions" xmlns="http://www.omg.org/spec/BPMN/20100524/DI" />
  </BPMNDiagram>
</definitions>

Or should it drop the unused / not needed bpmndi namespace?

<?xml version="1.0" encoding="utf-8" standalone="yes"?>
<definitions id="Definitions" xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" targetNamespace="http://bpmn.io">
  <BPMNDiagram id="BPMNDiagram_1" xmlns="http://www.omg.org/spec/BPMN/20100524/DI">
    <BPMNPlane bpmnElement="Definitions" xmlns="http://www.omg.org/spec/BPMN/20100524/DI" />
  </BPMNDiagram>
</definitions>

falko · 2020-04-13T22:02:24Z

Ideally, it looks like the imported: no namespace prefixes but xmlns attributes on elements. A bonus would be to remove unused namespace declarations.

If it is too hard to remember if the DI elements were prefixed or not, we could also export them always with prefix but also declare it properly.

nikku · 2020-04-14T06:42:08Z

Thanks! Your feedback helps to move towards a usable solution.

falko · 2020-04-14T10:36:27Z

Just a thought: Maybe filtering out unused namespace prefix declaration before the import could help to focus on the right prefixes to remember for the export.

nikku · 2020-04-14T10:55:44Z

Just a thought: Maybe filtering out unused namespace prefix declaration before the import could help to focus on the right prefixes to remember for the export.

This is an implementation detail we don't have to care about at this point. What is more important is the what and you've helped us tremendously setting that straight.

When it comes to fixing this bug: We've queued it into our "tasks to work on" topic list. We do not have any concrete plans when it will be fixed.

falko · 2020-04-14T11:02:06Z

If you can not fix it any time soon, what could be a workaround, i.e. what could we ask CaseAgile to change in their export? Would it help if their tool would prefix the DI elements? It could save some file size, given how often the xmlns attribute has to be put in the DI section of the XML.

nikku · 2020-04-14T11:04:21Z

What is your time line, i.e. when would you need a fix?

We'll not fix this right now but we'll look into this in an orderly manner, in the near future.

nikku · 2020-04-14T11:07:24Z

CaseAgile could export their diagrams in the normal manner:

<definitions xmlns:bpmndi="...">
  <bpmndi:BPMNDiagram>...</bpmndi:BPMNDiagram>
</definitions>

Rather than attaching the namespace on each element indivually AND writing the xmlns:bpmndi namespace information:

<definitions xmlns:bpmndi="...">
  <BPMNDiagram xmlns="...">...</bpmndi:BPMNDiagram>
</definitions>

But it is really a bug on our side we need to fix, too.

This ensures that we always use the inner most form for namespace declarations in cases where namespaces are declared multiple times, with different prefixes: ```xml <root:complexNesting xmlns:root="http://properties" id="ComplexNesting"> <complexNesting xmlns="http://properties"> <complexNesting> <foo:complexNesting xmlns:foo="http://properties" /> </complexNesting> </complexNesting> </root:complexNesting> ``` Outer, unused declarations are still cleaned up accordingly, as this is our existing behavior. This means that we never export something like this (because we strip the global, unused declaration): ```xml <root xmlns="http://extended" xmlns:unused="http://properties" id="Root"> <base xmlns="http://properties" /> </root> ``` Related to bpmn-io/bpmn-js#1310

This adds a test that verifies that locally re-declared namespaces are kept (to keep diagram changes to a minimum). Related to bpmn-io/bpmn-js#1310

This ensures that we always use the inner most form for namespace declarations in cases where namespaces are declared multiple times, with different prefixes: ```xml <root:complexNesting xmlns:root="http://properties" id="ComplexNesting"> <complexNesting xmlns="http://properties"> <complexNesting> <foo:complexNesting xmlns:foo="http://properties" /> </complexNesting> </complexNesting> </root:complexNesting> ``` Outer, unused declarations are still cleaned up accordingly, as this is our existing behavior. This means that we never export something like this (because we strip the global, unused declaration): ```xml <root xmlns="http://extended" xmlns:unused="http://properties" id="Root"> <base xmlns="http://properties" /> </root> ``` Related to bpmn-io/bpmn-js#1310

Related to bpmn-io/bpmn-js#1310

This adds a test that verifies that locally re-declared namespaces are kept (to keep diagram changes to a minimum). Related to bpmn-io/bpmn-js#1310

Related to bpmn-io/bpmn-js#1310

This ensures that we always use the inner most form for namespace declarations in cases where namespaces are declared multiple times, with different prefixes: ```xml <root:complexNesting xmlns:root="http://properties" id="ComplexNesting"> <complexNesting xmlns="http://properties"> <complexNesting> <foo:complexNesting xmlns:foo="http://properties" /> </complexNesting> </complexNesting> </root:complexNesting> ``` Outer, unused declarations are still cleaned up accordingly, as this is our existing behavior. This means that we never export something like this (because we strip the global, unused declaration): ```xml <root xmlns="http://extended" xmlns:unused="http://properties" id="Root"> <base xmlns="http://properties" /> </root> ``` Related to bpmn-io/bpmn-js#1310

nikku · 2020-04-16T18:35:32Z

... (1) and (2).

Related to bpmn-io/bpmn-js#1310

ChrisSchoe · 2020-04-27T07:51:23Z

Just fyi, we'd also be interested in an eventual fix for this issue. Our viadee BPMN modeler for Confluence also uses bpmn-js and it'd make our participation in the miwg demo much easier.

oguzeroglu · 2020-04-27T08:26:31Z

We're working on a fix and planning to merge the fix within this week.

Related to bpmn-io/bpmn-js#1310

Closes #1310

Related to bpmn-io/bpmn-js#1310

Closes #1310

oguzeroglu · 2020-04-28T06:59:03Z

Closed via #1317

falko · 2020-04-28T18:01:13Z

Awesome! Thank you!

linus-amg added the bug Something isn't working label Apr 9, 2020

nikku added a commit to bpmn-io/moddle-xml that referenced this issue Apr 10, 2020

test(writer): add failing namespace serialzation test case

e1af6c4

Related to bpmn-io/bpmn-js#1310

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 10, 2020

test(write): verify namespace serialization error

a36fa0a

Related to bpmn-io/bpmn-js#1310

nikku added the ready Ready to be worked on label Apr 10, 2020

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 14, 2020

test(roundtrip): verify local namespaces are kept

2a2fcde

This adds a test that verifies that locally re-declared namespaces are kept (to keep diagram changes to a minimum). Related to bpmn-io/bpmn-js#1310

nikku mentioned this issue Apr 14, 2020

Correctly serialized nested local namespaced elements bpmn-io/moddle-xml#47

Merged

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 14, 2020

test(roundtrip): verify local namespace declaration export

333eaf1

Related to bpmn-io/bpmn-js#1310

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 14, 2020

test(roundtrip): verify local namespaces are kept

0c4b91c

This adds a test that verifies that locally re-declared namespaces are kept (to keep diagram changes to a minimum). Related to bpmn-io/bpmn-js#1310

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 14, 2020

test(roundtrip): verify local namespace declaration export

2d983c8

Related to bpmn-io/bpmn-js#1310

This was referenced Apr 14, 2020

Verifies nested local namespace serialization bpmn-io/bpmn-moddle#75

Merged

Verify nested local namespace declaration (6.x backport) bpmn-io/bpmn-moddle#76

Merged

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 16, 2020

test(edit): verify missing namespace export

3948807

Related to bpmn-io/bpmn-js#1310

nikku added the ready Ready to be worked on label Apr 20, 2020 — with bpmn-io-tasks

oguzeroglu self-assigned this Apr 21, 2020

oguzeroglu added in progress Currently worked on and removed ready Ready to be worked on labels Apr 21, 2020

oguzeroglu added a commit to bpmn-io/moddle-xml that referenced this issue Apr 27, 2020

feat(write): inform parent NS when logging an NS as used

1318c29

Related to bpmn-io/bpmn-js#1310

oguzeroglu mentioned this issue Apr 27, 2020

Reuse global namespace bpmn-io/moddle-xml#49

Merged

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 27, 2020

test(edit): verify missing namespace export

f409d05

Related to bpmn-io/bpmn-js#1310

nikku pushed a commit to bpmn-io/moddle-xml that referenced this issue Apr 27, 2020

fix(write): recursively log NS as used

8db96f8

Related to bpmn-io/bpmn-js#1310

nikku pushed a commit to bpmn-io/moddle-xml that referenced this issue Apr 27, 2020

fix(write): recursively log NS as used

7a31880

Related to bpmn-io/bpmn-js#1310

nikku pushed a commit to bpmn-io/moddle-xml that referenced this issue Apr 27, 2020

fix(write): recursively log NS as used

cb3a4f5

Related to bpmn-io/bpmn-js#1310

nikku mentioned this issue Apr 27, 2020

No namespace reuse bpmn-io/bpmn-moddle#78

Merged

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 27, 2020

test(edit): verify missing namespace export

c0cc04b

Related to bpmn-io/bpmn-js#1310

nikku added a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 27, 2020

test(edit): make fail-safe

720b70b

Related to bpmn-io/bpmn-js#1310

nikku added a commit that referenced this issue Apr 27, 2020

chore(project): bump bpmn-moddle

433e860

Closes #1310

bpmn-io-tasks bot assigned nikku Apr 27, 2020

nikku mentioned this issue Apr 27, 2020

chore(project): bump bpmn-moddle #1317

Merged

bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Apr 27, 2020

fake-join bot pushed a commit to bpmn-io/bpmn-moddle that referenced this issue Apr 28, 2020

test(edit): verify missing namespace export

1211bda

Related to bpmn-io/bpmn-js#1310

fake-join bot pushed a commit that referenced this issue Apr 28, 2020

chore(project): bump bpmn-moddle

f4f42ab

Closes #1310

oguzeroglu closed this as completed Apr 28, 2020

bpmn-io-tasks bot removed the needs review Review pending label Apr 28, 2020

nikku added export miwg Tracks BPMN compatibility issues labels Apr 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Namespaces get removed when exporting foreign diagram #1310

Namespaces get removed when exporting foreign diagram #1310

linus-amg commented Apr 9, 2020 •

edited

Loading

nikku commented Apr 10, 2020

falko commented Apr 13, 2020

falko commented Apr 13, 2020

nikku commented Apr 13, 2020

nikku commented Apr 13, 2020

falko commented Apr 13, 2020

nikku commented Apr 13, 2020

falko commented Apr 13, 2020 •

edited

Loading

nikku commented Apr 14, 2020

falko commented Apr 14, 2020

nikku commented Apr 14, 2020

falko commented Apr 14, 2020

nikku commented Apr 14, 2020 •

edited

Loading

nikku commented Apr 14, 2020

nikku commented Apr 16, 2020 •

edited

Loading

ChrisSchoe commented Apr 27, 2020

oguzeroglu commented Apr 27, 2020

oguzeroglu commented Apr 28, 2020

falko commented Apr 28, 2020

Namespaces get removed when exporting foreign diagram #1310

Namespaces get removed when exporting foreign diagram #1310

Comments

linus-amg commented Apr 9, 2020 • edited Loading

nikku commented Apr 10, 2020

falko commented Apr 13, 2020

falko commented Apr 13, 2020

nikku commented Apr 13, 2020

nikku commented Apr 13, 2020

falko commented Apr 13, 2020

nikku commented Apr 13, 2020

falko commented Apr 13, 2020 • edited Loading

nikku commented Apr 14, 2020

falko commented Apr 14, 2020

nikku commented Apr 14, 2020

falko commented Apr 14, 2020

nikku commented Apr 14, 2020 • edited Loading

nikku commented Apr 14, 2020

nikku commented Apr 16, 2020 • edited Loading

ChrisSchoe commented Apr 27, 2020

oguzeroglu commented Apr 27, 2020

oguzeroglu commented Apr 28, 2020

falko commented Apr 28, 2020

linus-amg commented Apr 9, 2020 •

edited

Loading

falko commented Apr 13, 2020 •

edited

Loading

nikku commented Apr 14, 2020 •

edited

Loading

nikku commented Apr 16, 2020 •

edited

Loading