-
Notifications
You must be signed in to change notification settings - Fork 748
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
Fixed an issue with import/export of multiple entities #5930
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to discuss this in the next TAG meeting. This would fix the issue at hand but I am not sure this is what we should do or find another way for that obsolete serialization higher up.
Agreed. Thanks so much for submitting this PR @zyhfish 🎉 Our next meeting is on Jan 23. |
Hi @valadas , another possible solution is handle the XmlSerializer.UnknownElement or UnknownNode event handler to add the obsoleted properties manually, but IMO this is not needed as we need to remove them finally in the result. |
HI @zyhfish my concern is not just about this specific bug but overall serialization by others on these objects. Being public and providing a base class any 3rd party could potentially have their own permissions inherited by one of these classes to do something custom and #5841 caused a breaking change in that respect. I took a stab at implementing IXmlSerializable and quickly discovered that it was a bad idea because then any subclass would call the highest inherited class implementation unless we override those methods in the whole chain of inheritance. So it would not serialize any lower details. I ran out of time to dig further, so I'll have to revisit this but curious about what people think on #5943 which has a different approach to simply having both casings present and making the wrong case property [CLSCompliant(false)] |
That being said I did not yet try UnkownElement or UnkownNode, not sure on that specific one but could be something as long as we don't have the same issue as in IXmlSerializable implementation |
Closes #5894
remove the child node with null reference check.