Missing width/height attribute for images when added via (image: ) / kirbytext

[stairjoke]

Description

When Kirby renders a field containing the kirbytext flavor of markdown into HTML, it does not add width and height attributes to the <img /> element. Besides their obvious functionality, these two fields inform the browser about an image‘s aspect ratio. Adding them will improve the experience for site visitors on bad/slow connections, and users of the kirbytext()-method can currently not add this themselves easily. I did not test the markdown()-method. If the lack of width/height attributes is considered a bug, the markdown()-method may need to be updated as well to be consistent.

I understand this might constitute a breaking change for some themes, as there will be the need to update some theme‘s CSS to include a height-attribute. Still, I think the improvement is worth considering. There is information about this in this older video from 2019: https://youtu.be/4-d_SoCHeWE

Expected behavior
Any (image: ) element contained in a Field should be rendered into HTML that includes the width and height attributes for that image. Considering images currently render a figure-element containing an img-element, I would expect this result:

<figure>
  <img alt="…" src="…" width="…" height="…">
</figure>

Alternative 1 I considered suggesting this alternative: Add style: "aspect-ratio: [width] / [height]" to all img-elements. Example:

<figure>
  <img alt="…" src="…" style="aspect-ratio: [width] / [height]">
</figure>

However, this would give the aspect-ratio the highest specificity and likely break more than adding the width/height attributes.

Alternative 2 Do nothing. I considered if adding width/height attributes would do more harm than good. I do not know for sure. As adding them is a best-practice to avoid reflows after the initial render of the page, I think the fact that they are missing is a bug.

To reproduce

1. Add an (image: ) to a kirbytext-field in a page‘s TXT file

example.txt

title: Example Page

----

text:

Nulla vitae elit libero, a pharetra augue. Nullam quis risus eget urna mollis ornare vel eu leo.

(image: image.png alt: An image)

Maecenas sed diam eget risus varius blandit sit amet non magna. Duis mollis, est non commodo luctus, nisi erat porttitor ligula, eget lacinia odio sem nec elit.

Your setup

Kirby Version
3.9.1

Your system (please complete the following information)

• Device: MacBook Pro
• OS: macOS 13.2.1 (22D68)
• Browser: Safari
• Version: Version 16.3 (18614.4.6.1.6)

[afbora]

I know that the width and height information of images and videos is especially important for CLS (Cumulative Layout Shift). I think we can make this a bit easier with the new auto or a new prop without the breaking change.

new `auto` feature:
(image: image.png alt: An image height: auto width: auto)

or new shortcut like `dimensions`:
(image: image.png alt: An image dimensions: true)

output will be for both:
<figure>
  <img alt="…" src="…" width="…" height="…">
</figure>

[texnixe]

The kirbytag already supports the width and height attributes, but of course, it would be a bit painful to have to add these manually. On the other hand, maybe not everyone wants to have these attributes added automatically. Therefore, some attribute that lets the user decide easily would probably make sense?

[lukasbestle]

I had completely missed that browsers now apparently support getting the aspect ratio from the attributes even when width: 100% is used in the CSS. Before that was the case, there was no real point adding the attributes because you will either have images that are displayed in their native (and therefore most likely wrong) unscaled size or overrides in the CSS that completely override the information from the attributes.

I wonder if there is any disadvantage of adding the attributes always and by default. There is the breaking change that relative units of the width or height CSS properties will need to be accompanied by the other property set to auto. But apart from that?

Because if there is no significant disadvantage, I'd vote to add the width and height attributes by default. If custom values are set in the tag, they will override the actual size. If just one of the values is set in the tag, the other one will be calculated based on the aspect ratio of the image. I think that would be really clean and editors wouldn't have to think about it.

[afbora]

As Lukas said, I would also vote in favour of rendering the width and height attributes by default, if it doesn't affect the appearance/view of existing installations.

[distantnative]

There is the breaking change that relative units of the width or height CSS properties will need to be accompanied by the other property set to auto. But apart from that?

@lukasbestle I don't understand this one, can you explain? Might just be me after a long day.

Cause I would also think by default could work.

[lukasbestle]

There is the breaking change that relative units of the width or height CSS properties will need to be accompanied by the other property set to auto. But apart from that?

@lukasbestle I don't understand this one, can you explain? Might just be me after a long day.

If you set the native image dimensions with the <img> attributes and use the following CSS:

img {
  width: 100%;
}

The width will be overridden but the height won't be. So the browser will use the absolute height from the attribute but scale the width dynamically. Thus the aspect ratio will be incorrect.

If you don't use the attributes (current behavior of the Kirbytag), the browser will keep the aspect ratio and scale the height dynamically as well.

But once you use the attributes, you need to set height: auto so that the aspect ratio is preserved. The same applies the other way around with width: auto if you use a relative height unit in CSS.

[distantnative]

Ah now got it, thanks! But this could be indeed a breaking change not to overlook. I'd think a lone width: 100% isn't so uncommon.

[afbora]

img {
  width: 100%;
}

It is also one of my frequent uses.

[stairjoke]

I find this discussion really helpful and interesting to read along, thanks for considering my report! My concern is that turning this on by default in a minor update to Kirby might put people off who bought a template or others who sell templates and receive unexpected support requests.

I‘ve been giving this further thought, and I think I have found a solution to the breaking-change-problem. I think going for a soft rollout as a config option in Kirby 3.X, and turning it on by default in Kirby 4 could be an option?

I think introducing two keywords for the config.php could be a solution: intrinsicHeight and intrinsicWidth. This would allow users to configure kirbytext to add the intrinsic width and height of an image by default, and it would allow “us” to easily set it as a default in Kirby 4.

return [
  'kirbytext' => [
    'image' => [
      'height' => intrinsicHeight,
      'width' => intrinsicWidth,
    ]
  ]
];

Adapted from: https://getkirby.com/docs/reference/system/options/kirbytext#image

Edit: I changed my proposal to use camel-case. I think Kirby should camel-case such keywords in general, but I believe that is a different discussion. It would make them easier to read and it would make them more accessible to people with limited eyesight.

[lukasbestle]

3.x releases are major releases (in the versioning scheme generation.major.minor.patch where 3 is the generation). Breaking changes in major releases are normal and we have those regularly. We try to reduce breaking changes as much as possible, but here it seems like there is no real long-term solution without a breaking change.

We could still introduce the new behavior with an option first (similar to what you proposed). This would allow devs to slowly migrate their sites and themes. However the syntax you suggested won't work. Like that they would be constants and would need to be all-caps and namespaced in a class. We could make them strings though.

[lukasbestle]

I noticed a second complication: The (image:) KirbyTag also supports external images by URL. We won't be able to determine the intrinsic size of those images without first fetching the image on the backend.

We could still support automatic width and height attributes for local files though. For external files it would be on the editor to add them manually to the tag.

Suggestion on a way forward:

• v4: Support for a special value 'intrinsic' (simple string) for the height and width attributes of the KirbyTag. Devs can set it for both attributes as default values in their config and already migrate their CSS where necessary.
• Also v4: Support for a special value 'none' that behaves the same as now. This value can be set by devs and/or editors where the behavior should stay the same even after v5.
• v5: 'intrinsic' becomes the new default and is no longer an option (if set anyway, it is ignored). 'none' is still an option for those who need the previous behavior.