HTML: remove early return in SVG processing#12425
HTML: remove early return in SVG processing#12425AA-Turner merged 5 commits intosphinx-doc:masterfrom
Conversation
|
You say that the tests pass but do we actually cover this case? If not, could you add a test covering the use of that option please? (just to see whether docutils is supporting the options correctly?) |
|
@picnixz I've added tests checking for the use of the |
|
thanks, In addition, to what @picnixz mentioned (understanding if the test cases cover the option use), |
|
In |
AA-Turner
left a comment
There was a problem hiding this comment.
Hi @tuncbkose, please could you add an entry to CHANGES? Otherwise looks good!
A
|
@AA-Turner rebased and added an entry to CHANGES. Thanks for the review! |
# Conflicts: # CHANGES.rst
|
@tuncbkose a tip for the future -- open a PR from a feature branch, rather than master -- makes things much easier! A |
This PR removes some code that was added a long time ago when
docutilsdidn't support SVGs. Removing this chunk passed all the tests.I encountered this when I was trying to use
:height:and:width:options with SVGs. Due to the removed code, if one specifies a unit, like5cm, it gets directly passed to the HTML attribute, which is incorrect. With this PR, the size handling is done bydocutils, which uses thestyleattribute where one can specify units.One thing to note is that
:scale:option for SVGs still doesn't work due to #12415. If this PR is accepted as it is, due to not returning early, the use of:scale:option for SVGs may emit an additional warning fromdocutils'sHTMLTranslator::image_sizeaboutPILbeing required. I'm not sure what is the best way to deal with that.Feature or Bugfix
Detail
:height:and:width:options for SVG files.