Image expanded capabilities: width & height, decoding, and loading

[joshbruce]

Been thinking about 3 areas related to images and the optimization project I'm doing...none of which are part of a Markdown spec; I don't think.

1. Image height and width
2. Image async via decoding attribute
3. Image loading attribute

All three (four) are already possible using the attributes extension. With that said I think the last two could be easily achieved through the config - false means don't bother, with either two or three possible strings. So, doesn't collide with or break spec compliance and no added syntax from the other, which would be required using the attribute extension. With that said, setting width and height is recommended if doing 2, 3, or both.

Image height and width could be derived using getimagesize and maybe adding a setting to the config to say whether you want this as part of the output - false by default; again, no deviation from spec and lets Markdown remain more for authors and light on syntax. (Could be a performance hit though.)

With that said, maybe I want to specify a different height and width in the rendered markup. I've seen a couple that can integrate seamlessly into the Markdown by appending it to the URL. One using a fragment and could be modified to seamlessly integrate, it's the {space}=WxH syntax. Though this would break with the specs, which don't support image width and height natively as part of the syntax.

Thoughts on any or all of these ideas??

I've been considering creating an extension and wondering what might be able to go directly into core.

[colinodell]

Firstly, let me say that I greatly appreciate the suggestions and discussion, as always!

1. Image height and width could be derived using getimagesize

Using getimagesize() as-is would (unfortunately) be a non-starter for me, as it supports any protocol or wrapper out of the box, and therefore could potentially be abused by evil Markdown input for nefarious purposes:

• Loading images over http(s) that take forever to load would tie up PHP-FPM processes and could be used to DoS a site
• Could be used to check for the existence of files on the server by trying different local paths and seeing if/when height and width attributes are added thus confirming file existence. Even if the file contents aren't exposed, this could be used for fingerprinting the CMS, OS, etc.
• Because it must load an entire image first, large images (or paths used maliciously like /dev/random) might result in long execution times and/or memory exhaustion issues
• zip:// with a ZIP bomb could exhaust memory
• Other theoretical evil ideas that I'm not clever enough to think of

Many of these issues could be mitigated with some additional input validation and setting some kind of timeout (is that possible with this function?), but even then it would be very difficult to properly and securely do all of that. So for those reasons, I'm inclined to decline that, but third-party libraries are certainly welcome to implement that at their own risk ☠️

2. Image async via decoding attribute

3. Image loading attribute

If you wanted to set those for all images via config, the Default Attributes extension is designed for exactly that purpose. Would that satisfy your needs? We could even include specific examples of those attributes in the docs to help others find that solution.

WDYT?

[joshbruce]

Always appreciate the conversations and considerations. :)

(Apologies for length, lots of whitespace and short lines. lol)

tl;dr

Fair and understandable on the getimagesize() - just a noodle against the wall and prefer a syntax-light method for the performance and customization perspectives.

My brain is coming from a different perspective as I'm the only author I need to worry about - any nefarious input would be mine, I think. lol (Would love to learn more about security implications on this score as I've never actually used getimagesize().)

If you wanted to set those for all images via config, the Default Attributes extension is designed for exactly that purpose. Would that satisfy your needs? We could even include specific examples of those attributes in the docs to help others find that solution.

To achieve the desired HTML output, yes. To achieve the authorship goals and desires, not preferred.

Update: Default attributes wouldn't work for the image size, but may for the loading and decoding options - sorry, didn't fully understand what you were saying there.

---

I tend to treat Markdown as a generic, plain text alternative for authors who want to avoid implementation details and learning syntax (light though it may be). So, not just HTML as a target and as syntax-light as possible for authoring.

Image is an odd duck in Markdown, so to speak. (Examples are just for illustration not education as you most likely know Markdown and the specs better than me. lol)

![Alt text](/path/to/image.png Title text)

I've seen people talk about this being the Markdown syntax they constantly need to look up (see Twitter search results). Some parsers even use it for video.

In my mind, using the attributes extension (Commonmark spec and Commonmark lib) begins to move away from the simple, don't-know-HTML (or development) and don't want to learn.

For the purposes of this discussion, as an author, I would need to know the target is HTML, what the attribute names are (width, height, loading, and decoding) and what possible values are available or valid - specific to the img element - then I have to write it.

![Alt text](/path/to/image.png title text){width="100px" height="100px" loading="lazy|eager" decoding="async|sync|auto"}

If I want to change or add a target that uses, say, different attribute names, I need to update, duplicate, or do both with the content.

(As a "lazy" author, I'd have to go through all my previous content and update it to add the attributes. Could also just set config to allow HTML to achieve the output, which means the Markdown becomes hardwired for an HTML target and I still need to go back and update all the entries with images.)

<img src="/path/to/image.png" title="title text" alt="Alt text" width="100px" height="100px" loading="lazy" decoding="async">

Alternatively (using attributes extension for width and height and the following config - proposal one, if you will - lol)...

$converter = new CommonMarkConverter([
  'img' => [
    'loading' => false|'lazy'|'eager', // ex. lazy (default is false)
    'decoding' => false|'async'|'sync'|'auto' // ex. async (default is false)
]);

![Alt text](/path/to/image.png title text){width="100px" height="100px"}

<!-- Same output -->

Makes loading and decoding a configuration detail specific to HTML output, instead of making it a content issue or consideration.

Image size seems appropriate for either an implementation detail for the parser (getimagesize() or similar) or a content/authorship detail - but not configuration because it's on an image by image basis.

The hangup is the specs don't inherently do width and height, so, third-party extension(s) makes sense, I think. One could support one or more of the "common" solutions (implemented by other parsers) and make it part of an extension-specific configuration. That would mean Commonmark wouldn't take care of the HTML note regarding width and height in conjunction with loading while leaving it open to the community to create and adopt extensions to their liking.

Of the two common ways referenced (without Commonmark extension usage), I appreciate the fragment addition the most. Mainly because it shouldn't break a parser that doesn't implement that consideration; some end up using it as the title because of the space between the URL and the next bit of text.

Putting it all together...

$converter = new CommonMarkConverter([
  'img' => [
    'loading' => false|'lazy'|'eager', // ex. lazy
    'decoding' => false|'async'|'sync'|'auto' // ex. async
    'size-unit' => false|'px'|'em'|'rem'|'%' // ex. px
]);

author...

![Alt text](/path/to/image.png#100x100 title text)

output...

<img src="/path/to/image.png" title="title text" alt="Alt text" width="100px" height="100px" loading="lazy" decoding="async">

Minimal burden and learning curve for author - extensive output for the target platform (HTML in this case). Parsers that don't know the syntax ignore it...drawback being a dead fragment, which I'm not sure if there are implications beyond HTML-craft (also removing loading and decoding capabilities from consideration) output would be:

<img src="/path/to/image.png#100x100" title="title text" alt="Alt text">

This is essentially the requirements document for half of the extension I've been thinking of writing and want to see what, if any of it, could be added to core before I run off and do the thing (if for no other reason than the learning as I'm sure the attribute extension is perfectly suitable for many users - otherwise this probably would have come up before). :)

Thanks for the consideration. Appreciate you and the lib a great deal - very welcoming and accessible on both counts.

[joshbruce]

ps. Commonmark community discussion has been on and off for 7 years - https://talk.commonmark.org/t/support-for-image-dimensions/148/45

[joshbruce]

All right, now I feel like a tool because I didn't differentiate between the Attributes extension and the Default Attributes extension (despite the link)...here we are. Happens and apologies again.

Yes, for the loading and encoding Default Attributes works; browser tested.

use League\CommonMark\Extension\CommonMark\Node\Inline\Image;

'default_attributes' => [
  Image::class => [
    'loading'  => 'lazy',
    'decoding' => 'async'
  ]
]

Re:

We could even include specific examples of those attributes in the docs to help others find that solution.

I think that would work. I'm also thinking it might be helpful to describe a bit more about how Default Attributes works from a specifying it perspective; honestly, it kinda frightened me a bit and I didn't fully understand it when I added it to the fluent markdown library. (I can see what I can do to help there, if you like.)

I'm presuming this would possibly apply to custom extension nodes as well - like the abbreviations one??

Definitely makes implementing the extension easier, I think. Now it's just the height and width piece.

Thanks again, as always!

[colinodell]

I'm also thinking it might be helpful to describe a bit more about how Default Attributes works from a specifying it perspective

I'm not the best at writing documentation, so any improvements would be great! 🙃

I'm presuming this would possibly apply to custom extension nodes as well - like the abbreviations one??

Yep! Any node type should work fine. It simply does a basic get_class() check for each node, sees if that node class exists in the configs, and applies any default attributes you provided.

commonmark/src/Extension/DefaultAttributes/ApplyDefaultAttributesProcessor.php

Line 37 in 1da87ad

if (($attributesToApply = $map[\get_class($node)] ?? []) === []) {

. (It does not currently support inherited types - for example, if you create a custom node type like class FancyImage extends Image, any default attributes defined for Image would NOT be applied - you'd have to specify FancyImage in that case)