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

[0.3.3] Started producing SVG without width, height and viewBox #61

Closed
hartwork opened this issue Feb 10, 2021 · 5 comments · Fixed by #62
Closed

[0.3.3] Started producing SVG without width, height and viewBox #61

hartwork opened this issue Feb 10, 2021 · 5 comments · Fixed by #62

Comments

@hartwork
Copy link
Collaborator

hartwork commented Feb 10, 2021

Hi @btel ,

due to my rendering regression tests in xiangqi-setup I noticed that things broke when trying to upgrade to svgutils 0.3.3. What happens is that the width, height, and viewBox parameters are no longer included in the written SVG file. Here's what that looks like in Inkscape: The dotted line is where the viewbox was with svgutils 0.3.2, the solid frame is where the new defacto viewbox is:

svgutils_0_3_3_viewbox

Here's a diff of before and after, with regard to the SVG output.

--- tests/actual-board-a4_blank_2cm_margin_BEFORE.svg   2021-02-10 11:22:04.300605915 +0100
+++ tests/actual-board-a4_blank_2cm_margin_AFTER.svg    2021-02-10 11:21:49.446494230 +0100
@@ -1,5 +1,5 @@
 <?xml version='1.0' encoding='ASCII' standalone='yes'?>
-<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="248.03149580999997px" viewBox="0 0 248.03149580999997 350.78740120838086" height="350.78740120838086px">
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1">
   <g>
     <g transform="translate(0, 0) scale(0.33333333299287476 0.33333333299287476) "><defs id="defs4"/>
   <sodipodi:namedview xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd" xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape" id="base" pagecolor="#ffffff" bordercolor="#666666" borderopacity="1.0" inkscape:pageopacity="0.0" inkscape:pageshadow="2" inkscape:zoom="0.7" inkscape:cx="359.38222" inkscape:cy="512.51453" inkscape:document-units="px" inkscape:current-layer="layer1" showgrid="false" inkscape:window-width="1920" inkscape:window-height="1024" inkscape:window-x="0" inkscape:window-y="0" inkscape:window-maximized="1"/>

I have a feeling that #58 is causing this: .width and .height (with no underscores) are no longer set?
Is that a bug or a feature? How can I make svgutils 0.3.3 put width, height and viewBox into SVG output again?

Thanks and best, Sebastian

@btel
Copy link
Owner

btel commented Feb 10, 2021

Thanks for the report, Sebastian. Indeed the width of the svg object was set using a property here:
https://github.com/btel/svg_utils/pull/58/files#diff-06c846d8d6ce5453fa5b4840a6650f74699cc55c16cde21e870df80c1f6cf220L254
which was removed in this PR. Please feel free, to open a PR to set the properties back on the object.

@hartwork
Copy link
Collaborator Author

Please feel free, to open a PR to set the properties back on the object.

I'm not too happy that it takes me to fix breakage like that but alright, I'll do it. And make the CI run the existing tests, if I can trust my eyes it's running black and doing releases but not running the test suite 🤔 Give me an hour or so.

hartwork added a commit to hartwork/svg_utils that referenced this issue Feb 10, 2021
…fixes btel#61)

Regression from commit 6f5d10f
introduce as part of pull request btel#58.
hartwork added a commit to hartwork/svg_utils that referenced this issue Feb 10, 2021
@hartwork
Copy link
Collaborator Author

I found the Travis tests now: They run so late that the commits I checked hadn't seen Travis CI, yet...
I'm happy to drop the GitHub Actions bytes from #62 if you want to stick with Travis. Alternatively, I can also drop Travis altogether, as you like.

@btel btel closed this as completed in #62 Feb 10, 2021
btel added a commit that referenced this issue Feb 10, 2021
…x-back-into-svg-output

Get width, height and viewBox back into SVG output (fixes #61)
@btel
Copy link
Owner

btel commented Feb 10, 2021

i just released v0.3.4 with your fix, thanks for reporting/fixing

@hartwork
Copy link
Collaborator Author

Awesome! I confirm that my regression tests are happy green with 0.3.4 now as well.

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 a pull request may close this issue.

2 participants