adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
730 stars 736 forks source link

Dropping image directly onto image component on the canvas results in empty alt text #2554

Open shilpaannastephen opened 1 year ago

shilpaannastephen commented 1 year ago

Bug Report

Current Behavior An empty alt tag is added to the image component if an image without a description is added directly into the image component. This issue can be reproduced by following below steps:

  1. Add an Image component to the page.
  2. Select any asset that is not updated with a description in its metadata. [example: /content/dam/we-retail/en/products/apparel/coats/Coats_men.jpg (from we-retail project)]
  3. Drop the image from the Assets side panel to the image component.
  4. When we click on the spanner icon of the image component, We can see the mandatory field 'Alternative text for accessibility *' is empty. Close the dialog.
  5. Open the page in published mode and inspect the image. The empty alt tag is present on the image tag.

The image component saves the image without alt text and an empty alt tag is added to the markup.

image image

The properties of the proxy component:

image

Expected behavior/code Empty alt tags should not be added to the image tag.

Environment

Possible Solution Do not allow images to be added to the component in any way if alt text is mandatory.

YuriyShestakov commented 1 year ago

The correct solution seems to be if the image is not marked as decorative and the altValueFromDam is not set to true but the required alt text is still empty, then act as if the altValueFromDam is set to true - get alt text from the asset description in DAM. This way it can be fixed in model. Or even easier - just add the altValueFromDam="{Boolean}true" as a default value to cq:template node for the image component. I would prefer fixing in both places, just in case.

shilpaannastephen commented 1 year ago

Thanks, @YuriyShestakov for the reply. The above scenarios work perfectly when the images have asset descriptions in DAM. But for the images that don't have asset description in DAM, an empty alt attribute is added to the image tag.

If we are authoring through the dialog editor, we have controls for switching off the alt tag if it is not required, adding a custom alt value, or adding alt text from the asset description in DAM. And, we can not close the dialog without choosing any of these options, which makes the author aware of their selections.

image

But, when the content author drags and drops the images directly into the image component, without opening the dialog, the image that doesn't have an asset description in DAM, leaves an empty alt attribute to the image tag without the knowledge of the content author.

Is there any way to restrict the images without asset description to be added to the image component, if the user is dragging and dropping the image directly into the image component?

YuriyShestakov commented 1 year ago

Hmm, sorry I forgot that you mentioned your image doesn't have a description in the metadata :( I have just opened and checked it in source code, haven't checked on AEM though, but there is a client library for image dialog (/apps/core/wcm/components/image/v2/image/clientlibs/editor/js/image.js), and in the js method retrieveDAMInfo(fileReference), when the description field is empty, it uses the dc:title field:

    if (altTuple) {
        var description = data["dc:description"];
        if (description === undefined || description.trim() === "") {
            description = data["dc:title"];
        }
        altTuple.seedTextValue(description);
        altTuple.update();
        toggleAlternativeFieldsAndLink(isDecorative);
    }

In your case, possibly both dc:description and dc:title are empty. If I am not mistaken it means either your image is too fresh - the workflows are still being executed (I am not sure how AEM allows you to drag that image in this case), or maybe that field uses only image metadata from EXIF (is not updated from the file name as a workaround), and that was empty in the original image. I would update that client library to use another field from the metadata in this case, if you have that. If there are no other fields that you can use, you can set the file name instead (get everything from the fileReference param after the last slash but before the last dot). Another possible way to solve your situation is updating one of the out-of-the-box image processing workflows to keep this field not empty (e.g. fill it from the file name).

Also, I see this validator in the same image dialog client library:

    $(window).adaptTo("foundation-registry").register("foundation.validation.validator", {
        selector: altInputSelector,
        validate: function() {
            var seededValue = $(altInputSelector).attr("data-seeded-value");
            var isAltCheckboxChecked = $(altCheckboxSelector).attr("checked");
            var assetWithoutDescriptionErrorMessage = "Error: Please provide an asset which has a description that can be used as alt text.";
            if (isAltCheckboxChecked && !seededValue) {
                return Granite.I18n.get(assetWithoutDescriptionErrorMessage);
            }
        }
    });

It looks like it must display the correct error in the dialog that the selected asset doesn't have an image description, which at least helps your authors to understand what is not correct with their image. The further step of this improvement would be creating/updating a client library for the authoring edit mode interface (not the dialog one), where this component will be marked as incorrectly configured in such cases, so authors will see the issue without opening the dialog, but I have concerns about including code like this one globally. Or you can add that error message into the HTML code of your overlayed image component for now.

Update. I just had a chance to check it on AEM, and it appeared I messed things up :) There's no altValueFromDAM checkbox in the dialog, but there is a checkbox about inheriting it from the page (altValueFromPageImage), and the mentioned checkbox is in the Design Dialog (component policies), which is true by default. So my previous suggestion about making it true through cq:template or another way does make absolutely no sense.