Provide better context in accessible names of EntityOperation views plugin drop-button operations

We already have #2960828: Link labels generated by the Entity List Builder should provide more context for a11y to address the drop buttons from EntityListBuilder, which will be a more predictable situation to compute names for.

@phenaproxima:

I would assume that this will make it easier for screen readers even if you’re using a table, right? That way you don’t have to traverse for context, since the table row isn’t labeled.

Yes, that's correct. The table row context I mentioned refers to "programmatically determined link context" in WCAG. Most screen readers provide tools for dealing with tables, like quickly jumping to the start/end of a row. But you can ask a screen reader to announce the headers for the current table cell, on demand, and you don't need to move away from the link. In the absence of explicit row-level headers, some screen readers will treat the first cell in the row as a header. I think it's wise to assume these tools are power-user level; not all screen reader users will be used to them.

Screen readers also have tools for listing links, or listing buttons. In this case, you don't have the benefit of the table row context. This issue can help screen reader users here.

We also have issues to make row-level headers too - #2962585: Treat entity label as a row-level header in entity list tables and #2770835: Add support for tables with two headers in views table display.

Following the same strategy as #2960828: Link labels generated by the Entity List Builder should provide more context for a11y adding a visually hidden text with the label of the entity.

This looks great!

Now we just need some test expectations to be updated, then it'll pass tests again :)

I'll work on this

Let's try to fix the tests

I reached a similar point fixing the tests so I think this is good to go.

+++ b/core/modules/views/tests/src/Functional/Handler/FieldEntityOperationsTest.php
@@ -76,7 +76,8 @@ public function testEntityOperations() {
-          $result = $this->xpath('//ul[contains(@class, dropbutton)]/li/a[@href=:path and text()=:title]', [':path' => $operation['url']->toString(), ':title' => (string) $operation['title']]);
+          list($operation_label) = explode(' ', strip_tags((string) $operation['title']), 2);
+          $result = $this->xpath('//ul[contains(@class, dropbutton)]/li/a[@href=:path and contains(text(), :operation)]', [':path' => $operation['url']->toString(), ':operation' => $operation_label]);

@chr.fritsch said he'd like to see this be more elegant. I did some digging.

I found https://stackoverflow.com/questions/16241197/xpath-to-retrieve-text-with..., which led to me trying the expression string-join(//span[@class="inner-span"]/text()) on their example, and that returns

    Text-data 1
    
     text Data 3
   

Which is pretty much what we want to use here.

Hope this helps!

The links will need to satisfy the new "Label in name" success criterion from WCAG 2.1. The short explanation is: don't put any visually-hidden text in between visible words in the link/button name. (So "Manage [Basic page] fields" would fail, for example.)

In this case, it's easy to satisfy WCAG because we only have single-word links to begin with. The changes to EntityListBuilder in patch #10 satisfy WCAG "Label in name" (for English, anyway - translators could cause a WCAG problem, so we'll need to provide guidance on this in the d.o translation handbook, say.). There's a plan to carry out an audit for WCAG "Label in name" across the whole of core, so we'll get another chance to confirm this anyway.

title tweak, a bit clearer about what the improvement is.

I think we need a change record here. It is highly likely that some contrib / custom tests are going to fail because of this sensible change so I think it is worth having so when that happens there's one to point at.

writing a change record now...

I didn't write a change record, it stlll needs work. Here's a new patch which updates some of the remaining entity operations, in classes which extend EntityListBuilder.

After looking again at #2960828: Link labels generated by the Entity List Builder should provide more context for a11y, I don't think we need both issues, we can do it all in one issue. Some things to consider:

The approach taken by the patch at #2960828-6: Link labels generated by the Entity List Builder should provide more context for a11y is to change the link title in EntityListBuilder::buildOperations(). It uses the operation title as visible text, followed by the entity label, then entity bundle label, in the visually-hidden span. This seems to work generically, but has a few issues. I use [square brackets] for the visually-hidden part:

• PRO: It catches all the entity operations for EntityListBuilder subclasses, including some added by contrib modules, like Devel. So we get a lot for free.
• PRO: Many of the resulting operation titles are good. Example "Edit [Basic page node_type]".
• CON: The format doesn't address the context of any particular operation titles, which may lead to some clumsy-sounding sentences, e.g. "Manage form display [Basic page node_type]"
• CON: Related, some operation titles have already been adjusted in sub-classes which override EntityListBuilder::getDefaultOperations(). For example MenuListBuilder changes the "Edit" operation title to "Edit menu". There could be some more unusual sentences resulting from this. An example is "Edit menu [User account menu menu]"
• CON: It doesn't modify the entity operations in Views, which is why we originally thought this would need two issues.
• OK: It currently assumes there is a bundle label, but not all entity types have bundles. In that case the bundle label is always the same as the entity type label, I think?

The approach in my patch here adds visually-hidden context to all the individual operations that are added by classes which extend EntityListBuilder::getDefaultOperations(), when they add their own operations. There are some issues here too:

• PRO: This allows us to tailor the operation titles better for individual EntityListBuilder subclasses, but...
• CON: We have to do it in every one of the EntityListBuilder subclasses.

This patch adds context to operations in the following classes:

• core/lib/Drupal/Core/Entity/EntityListBuilder.php
• core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php
• core/modules/menu_ui/src/MenuListBuilder.php
• core/modules/search/src/SearchPageListBuilder.php
• core/modules/action/src/ActionListBuilder.php

I stopped updating them after I had seen a few. I figured I would report back here, to get feedback about this! So what do people think about the approaches from these two issues? So far it's only considering how the core English versions are phrased.

TODO - the following classes still need visually-hidden link context added in their own getDefaultOperations() methods:

• core/modules/image/src/ImageStyleListBuilder.php
• core/modules/taxonomy/src/VocabularyListBuilder.php
• core/modules/views_ui/src/ViewListBuilder.php
• core/modules/block_content/src/BlockContentTypeListBuilder.php
• core/modules/field_ui/src/FieldConfigListBuilder.php
• core/modules/node/src/NodeTypeListBuilder.php
• core/modules/block/src/BlockListBuilder.php
• core/modules/shortcut/src/ShortcutSetListBuilder.php
• core/modules/workspaces/src/WorkspaceListBuilder.php
• core/modules/responsive_image/src/ResponsiveImageStyleListBuilder.php
• core/modules/user/src/RoleListBuilder.php
• core/modules/filter/src/FilterFormatListBuilder.php
• core/modules/comment/src/CommentTypeListBuilder.php

Oops, forgot to attach my patch...

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityListBuilder.php
@@ -35,14 +35,14 @@ public function getDefaultOperations(EntityInterface $entity) {
+          'title' => $this->t('Enable <span class="visually-hidden">@label</span>', ['@label' => $entity->label()]),

I have a concern about this. Build sentences from already translated labels often breaks. For example if the label is What should you do about a problem called potatoes then this will read Enable What should you do about a problem called potatoes. It looks like we have a couple of problems here. One is capitalisation ie Enable What the other is it possible to imagine things that very confusing. Say the title is Enable people to learn more and then you'll have Enable Enable people to learn more

Is quoting a good practice here like Enable "Enable people to learn more" ?

We've done that for things like

$this->t('Display "@display" needs a selected search fields to work properly. See the settings for the Entity Reference list format.', ['@display' => $this->display['display_title']]);

and

$this->t('Placeholder for the "@block" block', ['@block' => $this->label()])

So yeah I think that that is way forward.

I attach a patch that changes all the operations to have the label between quotes and fixes the tests (hopefully).

Re #25 - thanks for the updated patch rodrigoaguilera. I'm travelling today so I don't have time to review it in detail.

Re #23 and #24...

The quotes idea is probably good for visible text. Likewise, other punctuation like dashes or colons could help. Italics are another method which can work visually.

But for text in a visually-hidden span, there is very little benefit to using common puntuation to indicate the entity label. Screen readers don't announce it, or give any special indication, for the most part. Some screen readers do offer user preferences to control the announcement of punctuation, but the default settings are generally silence for common punctuation. The upshot is that we can't expect screen reader users to notice quote marks.

The thing that makes this tricky, is that we have no control over the phrasing in entity labels created by users - but these are the contextual information we want to convey in this issue.

The latest patches here have phrasing for the particular entity lists, but there will always be clumsy sentences this way.

So the important question is whether:

• it's worth tailoring the operation names, or
• going for a generic approach of operation name followed by entity label.

Can you provide an example about tailoring the the operation names. Is not very clear to me.

Attached is another attemp at fixing the tests.

I'm also not sure what that is asking exactly.

Here is the proper patch. The interdiff is the same

The link Set "?!`U>&`A" as default was not found on the page.

This seems like an encoding issue.

Let’s use a random string that does not contain ampersands and other characters that are HTML-encoded. That will likely fix this.

(Typing on the subway, unable to actually test this.)

Is not encoding, is a consistent fail because we are introducing HTML in the middle of the sentence that will be stripped by xpath text() method.

I changed the clickLink() for an xpath that searches for the two text objects that exist in that element in the right order. I can't think of a way of passing those throgh t().
Maybe in this case is better to create an xpath that translates to "click on the third link of the dropdown ul". What do you think?

I definitely learned a lot of xpath this sprint week :)

ugh, bad patch again. Now I am ignoring that folder

+++ b/core/modules/action/src/ActionListBuilder.php
@@ -99,7 +99,10 @@ public function buildHeader() {
+    if (isset($operations['delete'])) {
+      $operations['delete']['title'] = t('Delete <span class="visually-hidden">action "@label"</span>', ['@label' => $entity->label()]);

I don't think we need to add this.

The 'delete' operation will have been added by the grandparent class EntityListBuilder, and we set the context there.

Re-roll patch created for 9.1.x, Please review.

Fixed more test, Please review.

Please ignore the above interdiff, check this.

Fixed the test, Please check and advise it is up to the mark or not.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Tagging for https://www.w3.org/WAI/WCAG21/Understanding/label-in-name.html