search

silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 65 forks source link

BUG Properly handle empty image attributes #474

Closed maxime-rainville closed 2 years ago

maxime-rainville commented 2 years ago

Fixes https://github.com/silverstripe/silverstripe-cms/issues/2489

The regenerate_shortcode handler is used to normalise the data sent to the WYSIWYG. Looks like it was set up to handle boolean attributes like <input required /> by copying empty the name of the attributes to the value when the value is blank. The problem is that this would render out [img title="title"] for empty attributes.

Empty attributes would normally be stripped out anyway ... this is only a problem when the short code has been generated from outside the WYSIWYG.

Image tag don't normally have "boolean" attributes like required, so I don't think we need to worry about those. When I tried manually putting one in, it stop recognising the short code altogether ... so if you did put a boolean attribute in there, it wouldn't ever hit this bit of code anyway,

michalkleiner commented 2 years ago

Could we add the required attribute to the test, too? Just to see that value-less attributes are still handled correctly, whichever way 'correctly' means (strip/leave/turn into empty value string).

maxime-rainville commented 2 years ago

When I add [image src="/assets/success.png" id="1" width="128" height="128" class="leftAlone ss-htmleditorfield-file image" title="" alt="" required], it does not get recognised has a shortcode at all ... with or without a PR. So regenerate_shortcode won't get called.

If we want to support boolean attributes in shortcodes, we probably need to fix it in ShortcodeParser first.

michalkleiner commented 2 years ago

That's slightly annoying — thanks for confirming that @maxime-rainville. All good here then.