Problem/Motivation
The ImageFieldFormatter requires alt and title added to drupal-entity, but this is not documented in the README.md and those tags aren't included in the list of attributes added when adding an entity embed on the editor/filter form.
Out of the box, a person who creates a file embed button will not know why their "alt" and "title" attributes aren't saving.
This took me some digging to figure out. If you use a file entity embed with the "Image" ImageFieldFormatter, you must manually update the Allowed HTML tags to allow "alt" and "title" attributes on the drupal-entity tag.
Proposed resolution
- Documentation should be added to the README.md.
- "alt" and "title" should be added to list of allowed attributes added when an entity embed is created
- validation should be added to assure if someone manually removes the required tag and attribute from the allowed_html, the filter/editor form throws an error.
Remaining tasks
- reviews
User interface changes
- form validation added to filter_format_edit_form
API changes
- alt and title tags added to list of allowed tags for drupal-entity
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | 3022768-23.patch | 14.81 KB | wim leers |
| #23 | interdiff.txt | 8.55 KB | wim leers |
| #21 | 3022768-interdiff--12-21.txt | 8.66 KB | oknate |
| #21 | entity-embed-alt-title-3022768-21.patch | 13.88 KB | oknate |
| #12 | entity-embed-alt-title-3022768-12.patch | 14.55 KB | oknate |
Comments
Comment #2
oknateComment #3
oknateComment #4
oknateComment #5
oknateHere's a patch that adds some documentation to the README.md and adds validation on the form "filter_format_edit_form".
Comment #6
oknateTo do:
- Since the patch is adding a check for the drupal-entity tag within allowed_html, I think it could be updated to check each required attribute of the drupal-entity tag.
- add test coverage
Comment #7
oknateOh, there's a way to add the alt and title to drupal-entity automatically when a button is added to the editor. Adding to this patch.
See #2554687: <drupal-entity> tag not being automatically added when an Entity Embed button is added to active editor toolbar for reference.
This kind of makes the validation I'm adding less useful. But I suppose someone could manually remove those, and they should be warned that the button they have enabled has a dependency on those attributes.
Comment #8
oknateAdding test coverage
Comment #9
oknateComment #10
oknateComment #11
oknateComment #12
oknateHere's a updated patch that throws out the ImageFieldFormatter specific validation, and adds validation for a base set of attributes on the drupal-entity tag.
I'm working on an another issue that will add support for the alt and title tags on media entity of source type "image", see #2924391: [media entities] Regardless of @EntityEmbedDisplay plugin, Media entities representing a `image/*` MIME type should be able to have a per-embed `alt` and `title`.
Comment #13
oknateComment #14
andrewmacpherson commentedComment #15
andrewmacpherson commentedcleaning up issues with this typo in the tag name
Comment #16
wim leersComment #17
wim leersHow exactly does this relate to #2924391: [media entities] Regardless of @EntityEmbedDisplay plugin, Media entities representing a `image/*` MIME type should be able to have a per-embed `alt` and `title`?
Comment #19
wim leersI marked #3036302: Make Image Title Required as a duplicate of this, but I'd like it to be considered here too: @alienzed wrote:
Comment #20
oknateI wouldn't say that this makes the title and alt mandatory. It validates that they are allowed attributes on the drupal-entity embed element. This is required for proper functioning of saving of alt and title when saving it to the entity embed.
This issue:
#2924391: [media entities] Regardless of @EntityEmbedDisplay plugin, Media entities representing a `image/*` MIME type should be able to have a per-embed `alt` and `title`
is more about having the alt and title available in the dialog, and how the dialog should behave.
There are two different types of behavior could be implemented:
If you're using media, in some cases you might want to update every instance's alt and title, in other use cases, you might want the alt and title to be overridable on per embed basis. It really depends on what the expected user wants. Patch #17 on issue 2924391 adds an option to either save it to the embed or update the original media image.
So you could say that 2924391 requires this patch (or requires someone to know they need to add alt and title to the allowed attributes on drupal-embed in the filter settings).
The latest patch here is just assuring that alt and title are properties that are available. If they aren't used, for example on a node embed, they won't be added to the embed. So again, I wouldn't say that this makes the title and alt mandatory. This is making them allowed out of the box.
Comment #21
oknateRerolling #12. The interdiff is a little messed up because of a merge conflict.
Comment #22
oknateBecause I keep stepping on my own toes, I'm creating a meta issue with some of my test updates:
#3055928: [META] 8.x-1.0-beta4 plan
Comment #23
wim leersCan we please clone you? 😀
I only found nits, which I all fixed:
Nit: camelCase when it shouldn't be.
This comment is outdated. Removed.
Nit: s/html/HTML/.
Language nits and 80 cols violation. Fixed.
Excellent work! 👏
Comment #25
wim leers🚢
Comment #26
oknateSweet!