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

Option parseImgDimensions is actually true by default despite doc, and also, when false still behaves as if true #984

Open
paviad opened this issue Jul 11, 2023 · 2 comments · May be fixed by #985

Comments

@paviad
Copy link

paviad commented Jul 11, 2023

Problem Description

The default behavior of markdown to html is to parse image dimensions despite options.parseImgDimensions being false, i.e. with the default options, the following markdown is valid, and converts to the following html:

![my image](./pic/pic1_50.png =100pxx20px)

![my image2][1]

[1]: ./pic/pic1_50.png =100pxx20px
<p><img src="./pic/pic1_50.png" alt="my image" width="100px" height="20px" /></p>
<p><img src="./pic/pic1_50.png" alt="my image2" width="100px" height="20px" /></p>

The Fix

Fixing this issue requires two things in my opinion:

  1. In order to not break existing usage, the default should be changed to true both in code and in the documentation.
  2. When the option is false image dimensions should be rejected

Desired behavior with parseImgDimensions set to false

The above markdown should be rejected as valid image markdown and converted to the following html:

<p>![my image](./pic/pic1_50.png =100pxx20px)</p>
<p>![my image2][1]</p>
<p>[1]: ./pic/pic1_50.png =100pxx20px</p>
@joshiayush
Copy link

@paviad In my opinion the desired behaviour with parseImgDimensions set to false shouldn't result in the following html:

<p>![my image](./pic/pic1_50.png =100pxx20px)</p>
<p>![my image2][1]</p>
<p>[1]: ./pic/pic1_50.png =100pxx20px</p>

It should rather just omit the dimensions and fill in the html with proper values while leaving the dimension attributes, like the following:

<p><img src="./pic/pic1_50.png" alt="my image" /></p>
<p><img src="./pic/pic1_50.png" alt="my image2" /></p>

Because the important point to note here is that if we're setting parseImgDimensions to false that means we want the image to be displayed in its original dimensions regarding of the dimensions provided in the markdown syntax.

Note, we still want the image to be displayed but, not with the dimension attributes.

And the fix you've provided here #985 (which is highly appreciated) does not let the image render rather it just puts the markdown syntax in its place which is not the desired behaviour of a markdown to html converter.

I hope you'll look into it and you'll try to come up with a different approach.

@paviad
Copy link
Author

paviad commented Jul 18, 2023

It is definitely a viable alternative to my design - I can make it so.

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