mmikkel / Retcon-Craft

A collection of powerful Twig filters for modifying HTML
MIT License
79 stars 9 forks source link

retconAutoAlt outputs base64 string when enabling the base64 encoded src attribute #55

Closed tyssen closed 1 year ago

tyssen commented 1 year ago

With this code:

  {{ entry.body|retconSrcset(
    [
      'w320',
      'w480'
    ],
    'img',
    '(min-width: 48em) 40vw, 90vw',
    true
  )|retconAttr('img', {
    loading: 'lazy'
  })|retconAutoAlt() }}

the value of the alt attribute is svg+xml;charset=utf-8,%3Csvg%20xmlns%3D%27http%3A%2F%2Fwww.w3.

If I take true out, then it changes to the file name of the image, e.g. File-name instead of File name which is the actual title of the image. I read in this issue that retconAutoAlt would default to using the title but it seems to use file name minus the extension instead.

mmikkel commented 1 year ago

The retconAutoAlt filter only works as expected when the img src is a Craft reference tag (which it should generally be, if entry.body is a Redactor or CKEditor field), because that's the only way Retcon can query for the actual asset. Without a reference tag, the filter will default to extracting the file name from the src attribute, and use that for the alt attribute.

All this to say that _you need to use the |retconAutoAlt filter before any filters like retconSrcset (because that will change the src attribute from the initial reference tag, to something else):_

{{ entry.body
  |retconAutoAlt
  |retconSrcset(
    [
      'w320',
      'w480'
    ],
    'img',
    '(min-width: 48em) 40vw, 90vw',
    true
  )
  |retconAttr('img', {
    loading: 'lazy'
  }) 
}}

That said, it seems that there might be a regression from #17, where Retcon isn't able to find the asset even when the src attribute is a valid reference tag. I'll dig into that and hopefully get a fix out shortly.

I also think it's a bit awkward that, when the src attribute is a base64 encoded string, Retcon still uses that for the alt attribute. I might change it so that if the src attribute is neither a reference tag nor something that looks like an actual file URL, it should just set the alt attribute to an empty value. To be honest, even using the file name for the alternative text is an anti-pattern; in terms of accessibility it's generally better to have empty alt tags instead of what in many cases will be a meaningless value (like DC234345.jpg or Screenshot-12), though I'd be hesitant to actually change that behaviour until the next major Retcon release.

tyssen commented 1 year ago

I've swapped the order of the filters around and now it's outputting the asset title, not the file name, so all good now! Thanks :)

mmikkel commented 1 year ago

Thanks for the follow-up, @tyssen. Good to hear that it worked out on your end.

I was actually not able to output the asset title (only the file name) myself when I briefly tested this just now, which is why I suspected a regression for #17.

Do you mind telling me which Craft and Retcon versions you are using?

tyssen commented 1 year ago

Craft 4.4.13 and Retcon 2.6.1.

mmikkel commented 1 year ago

Thanks! I'll do some more thorough testing when I have the time, and will update or close this issue at that point.

mmikkel commented 1 year ago

I've added some guard rails against base64 placeholders becoming alt text now. Additionally, I've added a new $allowFilenameAsAltText param to the retconAutoAlt filter, which can be used to prevent filenames from being used as alt text altogether. In Retcon 2.x this parameter defaults to true for backwards compatibility, but the plan is to have it default to false in Retcon 3.0.

These changes will be in the next Retcon 2.x release.