Fix problems with text in SVG pictures #694
Conversation
|
SonarCloud doesn't like me leaving an unresolved TODO. // TODO: Consider higher resolution. PowerPoint seems to rasterize SVGs at 384-dpi, so 4x
// our dpi.This could be moved to a (very low priority) issue if you prefer. |
I have to admit that I agree with SonarCloud. Unfortunately, very often TODO items live forever in code if they are not turned into an issue with at least some minimal description. |
Ok, is done. |
| @@ -415,6 +416,47 @@ public void AddPicture_too_large_adds_svg_picture() | |||
| pres.Validate(); | |||
| } | |||
|
|
|||
| [Test] | |||
| [Explicit] | |||
| public void AddPicture_svg_with_text_matches_reference() | |||
There was a problem hiding this comment.
What about such edits?
[Test]
public void AddPicture_svg_with_text_matches_reference()
{
// ARRANGE
// This presentation contains the same SVG we're adding below, manually
// dragged in while running PowerPoint
var pres = new Presentation(StreamOf("055_svg_with_text.pptx"));
var shapes = pres.Slides[0].Shapes;
var image = TestHelper.GetStream("1x1.svg");
image.Position = 0;
// ACT
shapes.AddPicture(image);
// ASSERT
var picture = (IPicture)shapes.First(shape => shape.Name.StartsWith("Picture"));
var xml = new XmlDocument { PreserveWhitespace = true };
xml.LoadXml(picture.SvgContent);
var textTagRandomChild = xml.GetElementsByTagName("text").OfType<XmlElement>().First().ChildNodes.Item(0);
textTagRandomChild.Should().NotBeOfType<XmlSignificantWhitespace>("Text tags must not contain whitespace");
// The above assertion does guard against the root cause of the bug
// which led to this test. However, the true test comes from loading
// these up in PowerPoint and ensure the added image looks like the
// existing image.
pres.Validate();
}Regard xml.GetElementsByTagName("text").OfType<XmlElement>().First().ChildNodes.Item(0),
I think using statements as for, if, etc. in unit test make the test much more difficult to understand. If there is a case when, for example, .ChildNodes.Item(0).Should().NotBeOfType<XmlSignificantWhitespace>() is true, but .ChildNodes.Item(1).Should().NotBeOfType<XmlSignificantWhitespace>() is false, we'd better let such an error appear in production😁 and we'd better write a separate test for such case with a better understanding of what's going on.
There was a problem hiding this comment.
If this is how you'd prefer to do it, that's fine. The facts here are that text elements should not have ANY children which are whitespace. In this particular case, when it was broken, items 0 and 2 were whitespace. If something broke in the future where just item 1 was whitespace, that still would be broken.
|
Fixes #681
As noted in the issue, the root cause was that
AddPictureSvg()re-writes the SVG stream. This introduced a problem in<text>tags. The solution is to simply save in PowerPoint the SVG data we were originally given.Also note that I was unable to craft an automated test for this. I did add a test which includes a manual verification step, and left it as
Explicit. If you want to hear the long list of ideas I tried, I will gladly share!Finally, I will mention that this may have some conflict with #691, which I can resolve when that lands.
PR-Codex overview
The PR focuses on adding support for SVG images with text in PowerPoint presentations.
Detailed summary
AddPictureSvgmethod to accept aStreamparameter for SVG images.