open-scd
open-scd copied to clipboard
XML Prolog is omitted in saved file
Describe the bug
When importing an ICD and an IID file into a new Edition 2 project and then saving, the XML prolog is omitted (this caused the GE Enervista software to crash). We think it is also non-compliant with Part 6, Ed 2.1, section 8.4.
To Reproduce
Steps to reproduce the behavior:
- Go to New Project
- Click on Edition 2
- On the menu go to Import IEDs
- Import both IEDs in the attached zip file
- Save the project
- Open in an XML editor
- Observe there is no XML prolog
Expected behavior
The XML prolog is always included:
Also
This doesn't seem to be related to the input files but rather the way we instantiate a "New Project".
The code looks like it supports the XML prolog, but perhaps it is not sufficient to generate it from a string?
export function newEmptySCD(id, versionId) {
const {version, revision, release} = supportedAttributes[versionId];
const markup = `<?xml version="1.0" encoding="UTF-8"?>
<SCL xmlns="http://www.iec.ch/61850/2003/SCL" ${version ? `version="${version}"` : ""} ${revision ? `revision="${revision}"` : ""} ${release ? `release="${release}"` : ""}>
<Header id="${id}"/>
</SCL>`;
return new DOMParser().parseFromString(markup, "application/xml");
}
In the Communication Export plugin I did:
const sclNamespace = "http://www.iec.ch/61850/2003/SCL";
const sclDoc = document.implementation.createDocument(sclNamespace, "SCL", null);
const pi = sclDoc.createProcessingInstruction("xml", 'version="1.0" encoding="UTF-8"');
sclDoc.insertBefore(pi, sclDoc.firstChild);
Which seems to generate the correct prolog.
I think this is because the XML prolog is not XML, strictly speaking. It has to be added in a different way, this seemed helpful to me:
https://stackoverflow.com/questions/50109538/altering-an-existing-xml-prolog-with-javascript
It is quite obscure but it is not the way the prolog is formed:
- Creating a new (empty) Edition 2 project and saving it, it contains the prolog
- When creating a new project and combining both on my Windows machine (using Chrome) it does not contain the prolog.
- When creating a new project and combining both on my Linux machine (using Firefox or Chrome) it does not contain the prolog.
- When creating a new project and only using the 'NR.icd' in "Import IEDs" it does not contain the prolog (Firefox)
- When creating a new project and only using the 'B30.iid' in "Import IEDs" it does not contain the prolog (Firefox)
- It appears that something about the combined file after "Import IEDs" is used is problematic.
- If I open either 'NR.icd' or 'B30.iid' and then save it (without using New Project) the prolog is present.
So it seems to be narrowing it down to something related to what happens in "Import IEDs"
I thought it might be the difference between importNode
/ cloneNode
but it doesn't appear to be (although it could be in the editing/action API).
I put a breakpoint on L440 before the action to import the IED occurred, and imported both IEDs.
https://github.com/openscd/open-scd/blob/e22e42db4d89ea0161b2ea3db3b7ea5f38761423/src/menu/ImportIEDs.ts#L433-L445
Before I get:
new XMLSerializer().serializeToString(this.doc).slice(0,100)
'<?xml version="1.0" encoding="UTF-8"?><SCL xmlns="http://www.iec.ch/61850/2003/SCL" version="2007" r'
after I get:
new XMLSerializer().serializeToString(this.doc).slice(0,100)
'<SCL xmlns="http://www.iec.ch/61850/2003/SCL" version="2007" revision="B" release="4" xmlns:xsi="htt'
I wonder if the new editing API has a similar problem, I'll look to see exactly where the breakage occurs in some time.
Suggestion: maybe we can add an automated test to this in order to see if the behavior is correct.
I think the core problem resides in the action API.
I'm not sure how best to diagnose it. A workaround which can readily be applied might be to do the following in the "Save Project" which is effectively hunt for the relevant prolog-related item if it's not there, add the prolog.
This would prevent further work on the old API:
export default class SaveProjectPlugin extends LitElement {
async run() {
if (this.doc) {
// at prolog if it's been stripped
if (this.doc.xmlEncoding === null) {
const pi = this.doc.createProcessingInstruction(
'xml',
'version="1.0" encoding="UTF-8"'
);
this.doc.insertBefore(pi, this.doc.firstChild);
}
If this is acceptable I can do a PR.
We decided to see if progress in using the new edit action API will resolve this in a short time (next couple of weeks).
The root cause is that at the moment in the action API we instantiate a new document constantly without adding the prolog (thanks @ca-d):
https://github.com/openscd/open-scd/blob/e22e42db4d89ea0161b2ea3db3b7ea5f38761423/src/Editing.ts#L427-L435
This was broken about 4-6 months ago as part of improving the update cycle for plugins.
export default class SaveProjectPlugin extends LitElement { async run() { if (this.doc) { // at prolog if it's been stripped if (this.doc.xmlEncoding === null) { const pi = this.doc.createProcessingInstruction( 'xml', 'version="1.0" encoding="UTF-8"' ); this.doc.insertBefore(pi, this.doc.firstChild); }
If this is acceptable I can do a PR.
I'm currently in favor of this solution as a quick fix, since technical questions have come up for tomorrow's refinement which bear on our edit action migration timeline. If you could find the time to submit it after all, I'd try to review it as quickly as possible. Thank you!
Hello there,
Thank you for opening this issue! We appreciate your interest in our project. However, it seems that this issue hasn't had any activity for a while. To ensure that our issue tracker remains organized and efficient, we occasionally review and address stale issues.
If you believe this issue is still relevant and requires attention, please provide any additional context, updates, or details that might help us understand the problem better. Feel free to continue the conversation here.
If the issue is no longer relevant, you can simply close it. If you're uncertain, you can always reopen it later.
Remember, our project thrives on community contributions, and your input matters. We're here to collaborate and improve. Thank you for being part of this journey!
We contemplate the benefits of having bots make people work.
This issue is still here even though the underlying issue is now resolved because the "workaround" code which ensures the prolog is there should be removed as it is most likely just cruft at this point. I think this means #1173 could be reverted and a few tests would be required to see this issue was still resolved.