Improve placeholder text for image upload

Hi @Angélique Valvandrin
Thank you for your contribution and welcome to your first patch!

Two thoughts about it:

1. I would say "for screen reader users" instead of "for people with disabilities" to be the most inclusive and the less offensive possible.
2. I think we should not use the placeholder for this because it's not its original purpose (giving examples of content) and it's often cut when the field is not wide enough. What about putting all this in the description?

I agree with both items mentioned by DuaelFr above. Maybe a description like "A short description of the image for screen reader users, often required for accessibility compliance."

Welcome to the wonderful world of patching! :)

Always good to see a new patch contributor! Review of patch #2:

1. improves SEO

   Text alternatives aren't for SEO - they're for people. It's true that search engines use the content, but they aren't the intended audience of the alt attribute. I'd prefer we don't mention SEO here, because I think it's a bit misleading. I've seen some truly terrible examples. keyword-stuffing in alt attributes, written by people whose sole concern was SEO.

2. Will also be used as a fallback in case the image does not load

   Hmm, screen readers announce the alt attribute regardless of whether the image has loaded.

Re: #4.2

I think we should not use the placeholder for this because it's not its original purpose (giving examples of content) and it's often cut when the field is not wide enough. What about putting all this in the description?

I agree, this isn't a good use of the placeholder attribute. Let's use the #description only.

Re: #5

A short description of the image ...

That's a start, but it only covers some situations. I like the phrase used in WCAG 1.1.1 Non-text content - a text alternative "serves the equivalent purpose".

often required for accessibility compliance

I don't like this, it reinforces the policy-checkbox mentality, and completely misses the point of what text alternatives are actually for: understanding. How about "important for accessibility".
Fun fact: the word "compliance" doesn't appear anywhere in the WCAG 2.0 rec.

Somewhere there's an issue about improving the help text for imagefield configuration too, but I can't find it. I'll link it here when I do, it would be good to get them consistent.

There's lots of potential to bikeshed on this phrasing, and I'm guilty as anyone ;-)

Something we can definitely go ahead with: remove the #placeholder attribute, just use #description.

Dropping by to leave a comment.. deaf people *do not* use screen reader.. it is mostly for blind and visually impaired users.

However I am cool with using "for people with disabilities" regardless. But still, I agree with comments #4, #5, and #6 above as it sounds more appropriate without discriminating anyone with disabilities.

Thanks for the patch!

Here's a patch combining the changes requested above.

The last patch looks good to me.
I am a bit surprised it doesn't break any test, though.


+ '#description' => $this->t('Short description of the image used by screen readers; important for accessibility.')


The alt attribute isn't just used by screen readers. It is displayed visually if the image isn't loaded.

I think this wording covers all the bases.

Review of patch #12:

1. The new phrasing is good IMO.

2. +      '#description' => $this->t('Short description of the image used by screen readers and displayed when the image isn\'t loaded; important for accessibility.'),
   

   The coding standards prefer to avoid escaping single quotes in translated strings. Let's convert this to the double-quoted form.

Here's a final version using double-quotes.

We already have text for this in the ImageWidget see

'#description' => t('This text will be used by screen readers, search engines, or when the image cannot be loaded.'),

If we used the exact same text then existing translations would work and they wouldn't have something new to translate. I think the fact it is required shows that it is important (for accessibility and other reasons) so we don't really need that bit.

+1 to making it a description so this help text remains even when editing an image via ckeditor.

Here's a patch with the requested wording.

Sorry, but #21-22 are going in the wrong direction. I agree it would be good to get these descriptions the same, but let's not use the existing ImageWidget description. Search engines aren't the intended audience for text alternatives. The ImageWidget description is misleading because it suggests you should write alternative text with search engines in mind.

Somewhere there is an issue to update the ImageWidget text too, I just can't find it. We could change both of them here, and close the other one later (when I find it....).

I think the fact it is required shows that it is important (for accessibility ...

Only if you already happen to know what the alt attribute means. In my experience content authors are often confused or misinformed about it's purpose.

... and other reasons)

Accessibility is the only reason for text alternatives - all of the benefits of text alternatives given in the HTML5 recommendation amount to accessibility. Search engines aren't mentioned at all in relation to text alternatives in either the HTML5 or WCAG recs.

Wow, the current ImageWidget description has been with us since the 5.x-2.5 release of imagefield module in contrib! I went looking for it, found the origin in #366078-1: improve ALT text field description...

Thanks I've updated it to:

"Alternate text is used to describe the image. This text will be used by screen readers, search engines, or when the image cannot be loaded."

Most people won't care about screen readers, but everyone will fill out their ALT text if it gets them some SEO. :)

So it really was a sly plan to encourage accessibility dangling an SEO carrot. To be fair, I've done this myself in the past, but these days I don't think it's responsible advice.

Updating issue summary.
I found another place which needs fixing, in the image field settings form (ImageType)

+1 for @andrewmacpherson's opinions on the matter. Here's a patch with all of the changes.

+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -263,7 +263,7 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
-      '#description' => t('The alt attribute may be used by search engines, screen readers, and when the image cannot be loaded. Enabling this field is recommended.'),
+      '#description' => t("Short description of the image used by screen readers and displayed when the image isn't loaded; important for accessibility."),

I'm not sure this checkbox description should be changed. This is about enabling the field so the text "Enabling this field is recommended." is important and directly ties to the description for the title_field checkbox.

I see what you mean alexpott. Here's a patch that should convey how enabling alt text is important.

Looks good to me :)

1. +++ b/core/modules/editor/src/Form/EditorImageDialog.php
   @@ -141,7 +141,7 @@ public function buildForm(array $form, FormStateInterface $form_state, Editor $e
   +      '#description' => $this->t("Short description of the image used by screen readers and displayed when the image isn't loaded; important for accessibility."),
   

   Small nitpick: "isn't" should be "is not" in all occurances.

   See the interface text guidelines
   on https://www.drupal.org/node/604342

yay for guidelines :)

Good job here! I think we are really close to a commit! :)

As a translator, I think these strings should be split so we could prevent translations duplication.

I suggest the three following strings:

1. Short description of the image used by screen readers and displayed when the image is not loaded.
2. Enabling this field is important for accessibility.
3. Enabling this field is recommended.

As an example:

'#description' => $this->t('Short description of the image used by screen readers and displayed when the image is not loaded.') . ' ' . $this->t('This field is important for accessibility.'),

Another nitpick: according to best practices, the strings which do not contain any single quote should not be encapsulated in double quotes.

@DuaelFr I think we can add translation for the whole paragraph of the description. Added the updated patch regarding quotes to keep the consistency.

Thanks!

Can we get an advice whether #38 is the way to go? I've always been told to keep sentences whole because of context. I do understand the benefit of splitting but couldn't find documentation on whether we should do that or not. Also haven't examples in core where we do that.

Also re: #38, concatenating two t() strings means we are dictating to translators about which order the sentences must presented. That sounds like an unnecessary constraint to me. I'm not a translator, but someone who is explained that flexibility here can be useful. I understand that's why we include links as placeholders inside the whole string instead of concatenating them. Core already has plenty of short descriptions with two sentences in one string.

If we stick with the single string approach in #36, it would be worth replacing the semicolon with a full-stop here:

Short description of the image used by screen readers and displayed when the image is not loaded; important for accessibility.

The semicolon is being used correctly here, but a lot of native English speakers aren't confident about them, and it feels a bit technical. A full stop could make it more readable English.

+1 to #42. Surely this is new. Core has plenty of examples where all string in one t()

RE #38, you're actually making 2 different translatable strings into 3 somewhat shorter ones. None of the strings is very long anyway. I think keeping them whole as 2 strings is probably better?

In any case, I would actually say to change this string:
Short description of the image used by screen readers and displayed when the image is not loaded; important for accessibility.
to either:
Short description of the image used by screen readers and displayed when the image is not loaded (important for accessibility).
or as kind of suggested in #42
Short description of the image used by screen readers and displayed when the image is not loaded. This is important for accessibility.

Looks good to me: Single strings, single quotes, no contractions, no semi colons.

I found the duplicate issue I mentioned in #6 & #23. I've closed that one and brought credits across.

Committed 36ff61a and pushed to 8.7.x. Thanks!