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
| Comment | File | Size | Author |
|---|---|---|---|
| #28 | eu_cookie_compliance.3379830.1.25.0.patch | 67.37 KB | codex42 |
| #24 | 3379830-a11y.patch | 66.09 KB | aaronpinero |
Issue fork eu_cookie_compliance-3379830
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
Comment #2
rar9 commentedhow to get this fixed?
Comment #3
marco.aresu commentedI created a patch to solve the problem.
Comment #4
svenryen commentedComment #5
svenryen commentedIs "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.
Comment #7
cobasa commentedPatch #3 works, but I agree with #5. New patch attached.
Comment #8
cobasa commentedChanged status from Needs work to Needs review.
Comment #9
yang_yi_cn commentedI'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 ?
Comment #10
bronismateusz commentedThe installation goes without a hitch, but in my case it doesn't help at all.
Drupal: 10.2.5
EU Cookie Compliance: 1.24
Comment #11
bronismateusz commentedDespite clearing the cache, the js script loaded without a fix. Now, I can now confirm that everything works correctly.
Comment #12
floown commented@bronisMateusz : what patch work for you, please?
Comment #13
floown commentedI confirm that #7 works like a charm.
Comment #16
heddnRolled the last patch into an MR. And added
Drupal.t()to the strings. Leaving at RTBC as my changes where minor.Comment #17
svenryen commentedHi 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?
Comment #18
heddnThe 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.
Comment #19
gorkagr commentedHi!
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
Comment #20
f0ns commentedThank you, the patch provided works great here!
The notice in https://pagespeed.web.dev is gone now and the accessibility score increased.
Thanks!
Comment #22
codebymikey commentedUpdated 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-labelin the GOV.UK design system and this blog post.They both do make use of
regionroles rather thanalertdialog, 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.
Comment #23
aaronpinero commentedI 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.
Comment #24
aaronpinero commentedIf 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.
Comment #25
rajeevkPatches of #22 or #24 doesn't work with v1.25.0
Thanks
Comment #26
floown commentedComment #27
randalv commentedI 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)
Comment #28
codex42 commentedThis one works for 1.25.0
Comment #29
floown commentedThe patch #28 works. Thanks.
A nice news can be the commit for a new version :)
Comment #30
f0ns commentedThe 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.
Comment #31
hannahdigidev commentedThe patch in comment #28 works for me too . Thanks for the work on this fix :-)
Comment #32
svenryen commentedComment #34
atowl commentedComment #35
atowl commentedComment #36
svenryen commented