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

Testcase and workaround for Issue 432 - two other test cases are affected #443

Closed
wants to merge 4 commits into from

Conversation

fsteimke
Copy link
Contributor

This contribution is mainly for the new testcase mediaobject.006 where SVG image dimensions are given in inch.

The real cause is a problem with the external function ext:image-metadata which ignores width and height when given as value with a unit.

A workaround for Issue 432 is included, so that scaling of an SVG Image where dimensions have units seems to be correct. This affects two other testcases: calloutlist.004 and calloutlist.005. For both the tests will fail, because the created result does not meet expectations. But the dimensions of the generated images are unchanged, so i would think that the generated results are still correct.

@ndw
Copy link
Contributor

ndw commented Jan 16, 2024

I took a different approach, fixing the underlying metadata parser to handle units. I sort of knew I wasn't doing that, but I didn't have a test image that used units and hadn't got around to manufacturing one.

One point for the future: your editing tool is making a lot of irrelevant changes in whitespace. That makes the diff quite difficult to read. If it's possible, it would be nice to avoid that.

@ndw ndw closed this in #450 Jan 16, 2024
@fsteimke
Copy link
Contributor Author

Sure, my change was only a workaround,I couldn't even find the sources for the metadata parser.

Whitespace: I am using Oxygen as XML Editor. I will do my very best to avoid whitespace changes in the future..

@fsteimke fsteimke deleted the 432-svg-dimensions-workaround branch January 18, 2024 04:24
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.

2 participants