SVG definitions have width in rem which throws console errors

[michjnich]

The problematic definitions are in this file: https://github.com/jazzband/django-debug-toolbar/blob/0a622dcab4c78ce0f1d339f0e3147a1023350efa/debug_toolbar/templates/debug_toolbar/includes/theme_selector.html#L5

As I understand it, SVGs cannot define height in em or rem, but require pixels.

The current status results in errors in the console on Firefox stating "Unexpected value 1rem parsing width attribute." - 6 errors in total, one for each SVG definition.

Tested on Edge too and that does not seem to produce the errors, so this looks to be browser dependent. Looking at similar issues on other repos, I suspect Safari would also show them, but I am not using apple so cannot verify this.

Happy to make the change and submit a PR if somebody can say which pixel size they would need.

[tim-schilling]

I'm not able to reproduce this with Firefox 126.0.1 or 127.0.1 on a mac

[tim-schilling]

I was able to reproduce on Safari.

A PR would be greatly appreciated. I don't think we should use pixels as that breaks the scalability. I think you could wrap the svg in an element that uses the CSS style of width: 1rem; height: 1rem;, then remove the width and height attributes from the svg element. You'll need to play around with how to get it to appear properly though, but that will handle the sizing issue.

[michjnich]

For reference, I was getting this on Firefox on the work laptop, which is currently 115.12.0esr, if that makes any difference. Windows 10 enterprise build 19045.4529, v22H2.

Anyway, I'll see if I can find some time to play with it - not sure if there are any knock on implications of wrapping the svg in a div, so that probably needs some "mucking around time" factored in and am quite busy the next few weeks. I take your point about pixels though.

[michjnich]

OK, I can't get the wrapping with a div to work and display properly for some reason.

I have discovered though that simply changing to use css properties rather than the svg height and width works, because SVG attributes are more strictly defined and teh CSS is a bit broader, so I think this is the best (and least intrusive solution).

So basically style="width: 1rem; height 1rem" - or adding an svg-icon class with these properties and using that.

I'll sort out a PR with a new class later today, unless there are any objections to this change.

Edit.

Actually, I might add it to this section of the css in toolbar.css ...

#djDebug[data-theme="light"] #djToggleThemeButton svg.theme-light,
#djDebug[data-theme="dark"] #djToggleThemeButton svg.theme-dark,
#djDebug[data-theme="auto"] #djToggleThemeButton svg.theme-auto {
    display: block;
    height: 1rem;
    width: 1rem; 
}

[tim-schilling]

Thanks Michael!