Image: Lightbox buggy retrieval of alt attribute and insufficient accessibility

[afercia]

Description

See also discussion on #53026

The image lightbox needs some fixes and improvements:

1
Bug: at this point:

gutenberg/packages/block-library/src/image/index.php

Lines 123 to 127 in 3e260aa

	$processor = new WP_HTML_Tag_Processor( $block_content );
	$aria_label = __( 'Enlarge image' );
	$alt_attribute = $processor->get_attribute( 'alt' );

the alt attribute is always null because the HTML processor points to the first element of the block content, which is a figure element: no alt attribute can be retrieved. As such, the button to open the lightbox is always labeled Enlarge image even when the image does have an alt attribute. Right now, the Enlarge image button doesn't tell anything about what the image is.

2
The button to open the lightbox should be moved after the image in the DOM order. This is important especially for screen reader users. In a linearized content navigation experienc, it makes sense to place the 'what' first and then the action button after.

3
I'd argue the Enlarge image button placement inside the <figure> element isn't great from a semantics perspective. Ideally, the button should be moved outisde of the figure.

4
I still see an empty role attribute in the DOM before the lightbox is open. This is invalid HTML. Although seems to me this does not have an actual impact on accessibility because the modal dialog is aria-hidden="true", WordPress has a tradition to always output valid markup on the front end. It would be great to focus more on the quality of the actual output markup.
Bad value for attribute role on element [div]

5
In the same way, an empty src="" attribute is output in the DOM. This is invalid HTML.
Bad value for attribute src on element [img]: Must be non-empty.
Edut: to clarify, this applies to the second image within the modal dialog, i.e. the 'scaled' one. Initially, the src attribute is empty and it gets populated only when opening the modal dialog and the second image gets loaded.

6
The div element meant to be used for the modal dialog of the enlarged image is either labelled Image or it is labelled witht he image alt attribute:
<div role="" ... aria-label="Image" ...
<div role="" ... aria-label="{image alt attribute}" ...
Some more meaningful labelling would be nice. This is the dialog element, not the image. The labelling should clarify what the dialog is about. Users click a button that says Enlarge image: {alt text} and they get a modal dialog that is only labelled Image.

7
The Close button target size is too small. With Twenty Twenty-Four is aproximately only 15 by 18 pixels.

8
The Close button dosn't visually expose its accessible name. All icon buttons must always visually expose their name in some way. In the editor, we use the Tooltip component for this reason and it's already a compromise between design and accessibility. If this isn't possible on the front end, then tbe button must have visible text.

9
The enlarged image doesn't scale when resizing the window or changing device orientation. See the attached GIF for better clarity.

10
Performance: with large images, the modal dialog contains two images: the responsive-image and the enlarged-image. While I do realize why two images are necessary, I'm not sure this is OK from a performance perspective, as there is always one image that is unnecessary given a certain viewport size.
Edit: after a testing session together with @carolinan @aristath and @SergeyBiryukov we noticed this. may be not an issue, as on page load only one image is loaded. The second image is loaded when opening the modal dialog.

Step-by-step reproduction instructions

• Add an image block to a post and set it to open in a lightbox.
• Publish the post and see the front end.
• Inspect the source, use the lightbox and check the points reported above.

Screenshots, screen recording, code snippet

When changing device orientation (or resizine the browser window), the image in the open modal dialog doesn't scale correctly. It is cut-off on the left and right edges:

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[artemiomorales]

@afercia Thanks for taking a thorough look at the lightbox and for opening this issue. I see that you've already opened a PR to begin addressing these issues; here I'll leave a couple of hopefully helpful note:

3
I'd argue the Enlarge image button placement inside the <figure> element isn't great from a semantics perspective. Ideally, the button should be moved outisde of the figure.

This one will be hard to achieve. The reason the button lives alongside the image is that we're unable to put the button beside the figure and enclose the two in a wrapping div; here's the related issue.

The render method requires that we return a single HTML structure, and from what I've seen, the figure needs to be the outermost element, otherwise we risk breaking styles and overall backwards compatibility with existing themes and plugins.

5
In the same way, an empty src="" attribute is output in the DOM. This is invalid HTML.
Bad value for attribute src on element [img]: Must be non-empty.
Edut: to clarify, this applies to the second image within the modal dialog, i.e. the 'scaled' one. Initially, the src attribute is empty and it gets populated only when opening the modal dialog and the second image gets loaded.

The line setting the src attribute this is right here. Perhaps it can be as simple as replacing the empty string with a placeholder.

[SantosGuillamot]

Thanks a lot for the feedback here as well 🙂

4
I still see an empty role attribute in the DOM before the lightbox is open.
5
In the same way, an empty src="" attribute is output in the DOM.

After this fix, I believe we can use null in the selector we are using for the role (here) and for the src (here).

9
The enlarged image doesn't scale when resizing the window or changing device orientation. See the attached GIF for better clarity.

Regarding this, we discussed it here, and it seems we decided to postpone it because it wasn't considered urgent at that time. Anyway, I think it should be possible to add an effect that is triggered when the width or height changes. I added this commit in an experimental branch to showcase what I mean. It seems to work but I didn't test it thoroughly.

[afercia]

After this fix, I believe we can use null in the selector

That's what I did in my PR, so it is also consistent with similar code for the navigation.

Regarding this, we discussed it #52765 (comment), and it seems we decided to postpone it because it wasn't considered urgent at that time

I'm not sure I understand how a feature can be released when the user experience is clearly subpar on some devices e.g. tablets. Also on desktop, when changing the browser window size. I don't think these are edge cases. Anyways, I'm leaving this out from my PR.

[SantosGuillamot]

That's what I did in my PR, so it is also consistent with similar code for the navigation.

Didn't see the PR, sorry. Glad you were able to fix it.

I'm not sure I understand how a feature can be released when the user experience is clearly subpar on some devices e.g. tablets. Also on desktop, when changing the browser window size. I don't think these are edge cases.

I'm not saying this doesn't need to be fixed. I believe it does. What I was trying to say is that, as it only happens when you resize the screen while the lightbox is opened, and it works fine after closing and opening again, it seems back then it was considered that there were more important bugs/functionalities to solve.

I'm happy to create a proper PR if @artemiomorales thinks the approach I suggested here makes sense.

[afercia]

The reason the button lives alongside the image is that we're unable to put the button beside the figure and enclose the two in a wrapping di

I understand the underlying issue. Still, that is not how a figure element is supposed to be used according to the HTML specs. I don't have a solution, but I'm not sure going intentionally against the specs is the right direction to go.

[afercia]

The line setting the src attribute this is right here. Perhaps it can be as simple as replacing the empty string with a placeholder.

What about a base64 encoded small gif (26 bytes)? Yes it would be 'loaded' but it would be very small.

enlargedImgSrc: ( { context } ) => {
	return context.core.image.initialized
		? context.core.image.imageUploadedSrc
		: 'data:image/gif;base64,R0lGODlhAQABAAD/ACwAAAAAAQABAAACADs=';
},

[artemiomorales]

I'm happy to create a proper PR if @artemiomorales thinks the approach I suggested here makes sense.

@SantosGuillamot Thanks for taking a look at this. The approach you have here looks good to me — opening a PR sounds great. I may have a small comment on the implementation, but we can review that once the PR is up 🙏

[artemiomorales]

What about a base64 encoded small gif (26 bytes)? Yes it would be 'loaded' but it would be very small.

@afercia That sounds good to me 👍

[SantosGuillamot]

Pull request created:

• Reset "Expand on click" image styles when window is resized #55077