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

Added missing initialization for entities in SvgNodeReader #519

Merged
merged 2 commits into from
Jul 21, 2019

Conversation

mrbean-bremen
Copy link
Member

@mrbean-bremen
Copy link
Member Author

@H1Gdev - please have a look1

@@ -18,7 +14,11 @@ internal sealed class SvgNodeReader : XmlNodeReader
public SvgNodeReader(XmlNode node, Dictionary<string, string> entities)
: base(node)
{
this._entities = entities;
_entities = entities;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_entities = entities ?? new Dictionary<string, string>();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks - will change it.

{
if (this._entities.ContainsKey(this.Name))
if (_entities.ContainsKey(Name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible fix is to change it to an Entities property, but is there a reason not to do so ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been a property in the first place, and the bug came from not using it. In this case I see no reason for a property - it only adds complexity instead of removing it (being private, not automatic, and both readable and writable).

@mrbean-bremen mrbean-bremen merged commit ae7564c into svg-net:master Jul 21, 2019
@mrbean-bremen mrbean-bremen deleted the entities branch July 21, 2019 16:47
mrbean-bremen added a commit that referenced this pull request Jul 23, 2019
@gvheertum
Copy link
Member

@mrbean-bremen I noticed that somewhere in these commits the netcore 2.2 TargetFramework was removed from the unit test project. Was this intentionally?

@mrbean-bremen
Copy link
Member Author

Oops - certainly not! I will check this.

@mrbean-bremen
Copy link
Member Author

Thanks - fixed now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullReferenceException in SvgNodeReader.ResolveEntity when opening SVG with entity definitions
3 participants