Hello,

Problem/Motivation

Elements with role="dialog" or role="alertdialog" do not have accessible names.

Here is the code served by the module:

ARIA dialog elements without accessible names may prevent screen readers users from discerning the purpose of these elements. Learn how to make ARIA dialog elements more accessible https://dequeuniversity.com/rules/axe/4.7/aria-dialog-name

Regards

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

floown created an issue. See original summary.

rar9’s picture

how to get this fixed?

marco.aresu’s picture

I created a patch to solve the problem.

svenryen’s picture

Status: Active » Needs review
svenryen’s picture

Status: Needs review » Needs work

Is "Popup text" really a self-explanatory label for the banner?

I would assume we need something that reflects on this being a "Cookie Privacy Banner" to be much better.

cobasa made their first commit to this issue’s fork.

cobasa’s picture

Patch #3 works, but I agree with #5. New patch attached.

cobasa’s picture

Status: Needs work » Needs review

Changed status from Needs work to Needs review.

yang_yi_cn’s picture

I'm not that much an expert on these a11y things but can we combine this one with #3444215: Accessiblity - Remove aria and alertdialog from module ?

bronismateusz’s picture

The installation goes without a hitch, but in my case it doesn't help at all.

Drupal: 10.2.5
EU Cookie Compliance: 1.24

bronismateusz’s picture

Status: Needs review » Reviewed & tested by the community

Despite clearing the cache, the js script loaded without a fix. Now, I can now confirm that everything works correctly.

floown’s picture

@bronisMateusz : what patch work for you, please?

floown’s picture

I confirm that #7 works like a charm.

heddn made their first commit to this issue’s fork.

heddn’s picture

Rolled the last patch into an MR. And added Drupal.t() to the strings. Leaving at RTBC as my changes where minor.

svenryen’s picture

Status: Reviewed & tested by the community » Needs work

Hi all!

Thanks for teaming up to improve the a11y functionality of the cookie banner.

I see that the code changed from (supposedly) saying that the label for the element can be found in the DOM element with the ID "popup-text", to a text string.

This removes the connection (if there ever was one) between "sliding-popup" and "popup-text". and introduces a new string with the content of "Popup text".

I need to check with somebody that is familiar enough with Web Accessibility to check if this is a desirable outcome.

@heedn, I see you rerolled the patch from https://www.drupal.org/project/eu_cookie_compliance/issues/3379830#comme... and didn't take into consideration the code contributed in https://www.drupal.org/project/eu_cookie_compliance/issues/3379830#comme... where an improvement was attempted. Could you either work to improve this, or let us know the logic behind this change?

I'm also wondering if the final merge request does what's best for accessibility, or if we need to cosunlt somebody that has accessibility as a field of profession. Are we okay that the functionality presumably changes from saying that the string is in #popup-text or do we want a generic "Cookie Privacy Banner" to be added?

heddn’s picture

The accessibility here is improved with the new patch. Some of links in #17 are a bit confusing, but I think the question is if we are following https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/al.... Do we need to combine the 2 issues together? The patch in #3444215: Accessiblity - Remove aria and alertdialog from module only removes a bunch of hunks, it doesn't really add any hunks.

I do know that the complaints by Lighthouse tests are much improved after applying the latest patch here.

gorkagr’s picture

Hi!

I came with this issue while doing an accessibility check on one of my sites taht uses the module and i came with a MR in a different issue (https://www.drupal.org/project/eu_cookie_compliance/issues/3472337#comme...)
That one is closed, but i see I ussed the other attribute missing rather than the one used in this issue.

Best

f0ns’s picture

Thank you, the patch provided works great here!

The notice in https://pagespeed.web.dev is gone now and the accessibility score increased.

Thanks!

codebymikey made their first commit to this issue’s fork.

codebymikey’s picture

Updated the MR incorporating feedback from comment #7.

It now uses more specific labelling to distinguish what the banner is for. - I initially had a different label for the withdrawal and popup banner, but that's probably not necessary.

And in terms of convention as to how to handle the banner. We have examples of using just the aria-label in the GOV.UK design system and this blog post.

They both do make use of region roles rather than alertdialog, so it's possible that also needs to be revisited according to best practices.

I've also attached a static patch that applies cleanly against the 1.24 version of the module, incorporating the #3350586: Declined cookies with top popup pull content out of the page patch.

aaronpinero’s picture

I think there are two issues with the current approach, at least as I understand how it's being implemented:

(1) The text for the alert dialog label should, at minimum, be configurable. Personally, I'd like to see that text be represented as a heading or text within the alert dialog itself, rather than using an aria-label attribute.

(2) Not sure why the aria-describedby attribute is being removed. From what I see on MDN (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/al...) and what I read elsewhere, the alert dialog should have BOTH the aria-describedby attribute and either an aria-labelledby or aria-label attribute.

I would suggest getting this fix out quickly to address the immediate a11y errors. A larger question is whether or not to use the alert dialog role at all. I've seen some best practice suggestions point to using role="region" or a section element with an aria-labelledby attribute (which is supposed to be treated as a region by default). But that should be a separate issue.

aaronpinero’s picture

StatusFileSize
new66.09 KB

If anyone is interested, I am attaching the patch I am using on my sites while this and related D.o issues are sorted. I am applying the patch against version 8.x-1.24 on Drupal 10.3.5.

rajeevk’s picture

Patches of #22 or #24 doesn't work with v1.25.0

Thanks

floown’s picture

Version: 8.x-1.24 » 8.x-1.25
randalv’s picture

Version: 8.x-1.25 » 8.x-1.x-dev

I can confirm the MR applies to 1.25.

I now get the aria-label="Cookie privacy banner" parameter in the popup's HTML tag.

(PS. Let's keep working in the MR -if anything needs to change- rather than attaching patches)

codex42’s picture

StatusFileSize
new67.37 KB

This one works for 1.25.0

floown’s picture

The patch #28 works. Thanks.

A nice news can be the commit for a new version :)

f0ns’s picture

The patch #28 works for me. I would like to thank you for this work!

A small nitpick, please use - instead of . in patch filenames in the future.

[module-name]-[short-description]-[issue-number]-[comment-number].patch
# e.g. some_module-some-bug-123456-3.patch
hannahdigidev’s picture

The patch in comment #28 works for me too . Thanks for the work on this fix :-)

svenryen’s picture

Status: Needs work » Reviewed & tested by the community

  • atowl committed c9734c5d on 8.x-1.x
    Issue #3379830 by codex42, accessible names on elements with role
    
    Fixes...
atowl’s picture

Status: Reviewed & tested by the community » Fixed
atowl’s picture

Status: Fixed » Closed (fixed)
svenryen’s picture

Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.