Remove redundant role="article" from <article> html tags

Thanks for the patch.

1) Reviewed the code and changes seem ok.

2) Patch applies cleanly:

[mac:kristen:drupal-9.1.x-dev]$ patch -p1 < 3117230-2.patch
patching file core/modules/node/node.module
Hunk #1 succeeded at 540 (offset 2 lines).

3) Searched the 9.1 dev code after applying the patch and there is this:

./core/themes/bartik/templates/comment.html.twig: <article role="article"{{ attributes.addClass(classes)|without('role') }}>

Should this be removed too since "article" is the default?

Whoops... unassigning. Also marking "Needs work" based on #4.3.

Hi @Kristen,
Yes, I checked and as mentioned in https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article
that role article belongs to "Implicit ARIA role" which will create redudant in some situations.

Rerolled patch from #2 with some modification mentioned at #4.3 .
Please review latest patch

+++ b/core/themes/bartik/templates/comment.html.twig
@@ -74,7 +74,7 @@
-<article role="article"{{ attributes.addClass(classes)|without('role') }}>
+<article {{ attributes.addClass(classes)|without('role') }}>

1. Twig attributes are rendered with a space prefix, so there is no need for the space between <article and {{ attributes.
2. The role attribute can now be printed, since it can no longer be added twice.

Thanks for the update. Reviewing changes in #12 and looking at feedback from #10.

1. Twig attributes are rendered with a space prefix, so there is no need for the space between

Covered by update.

2. The role attribute can now be printed, since it can no longer be added twice.

Sorry, I don't understand this one.

Ah, I think I figured it out. The interdiff in #7 doesn't have it but the patch does. That confused me because I thought it was already gone. Thanks for the clarification.

I think this just needs some manual testing now.

The role="article" attribute was added for backwards compatibility with older browsers, specifically IE8 and earlier. Since we no longer support those browsers, this attribute can now be removed.

See browser support for <article> at https://caniuse.com/#feat=html5semantic

@idebr I'm confused because you added patches and marked RTBC. Also, I think this needs manual testing, no?

To be clear, for manual testing, my thought was just to make sure the markup doesn't have the role. Unfortunately, I don't have time at the moment to double check.

I've done the manual testing to check it doesn't show up for the node or the comment.

Screenshots without patch:

Screenshots with patch:

This change looks good. I wonder if it is worth a change record? It's possible that someone has changed there node.twig.html to not use the article tag right? It's probably extremely rare and not something that should prevent the change but still informing themers seems the right thing to do.

Also contrib code like https://git.drupalcode.org/project/adaptivetheme/-/blob/8.x-3.x/at_core/... is interesting as that adds the role. It'd be good to sure that browsers and accessibility tools are at one with the mozilla docs and W3 validator.

For the record we have many other redundant usage of role attribute.

• header role="banner"
• main role="main"
• footer role="contentinfo"
• aside role="complementary"
• nav role="navigation"
• summary role="button"

@chi - thanks for tagging this.

We should NOT remove the role="article" attribute from <article> for as long as we support IE11. I'm aware of the other roles mentioned in #24 - these should also remain. There is already a postponed issue to remove the landmark roles - #2655794: Remove redundant WAI-ARIA role attributes from <main>, <nav>, <aside>, <header>, and <footer> elements.

The reason for using the explicit ARIA roles, is that IE11 doesn't correctly map these implicit roles to the accessibility tree, and hence doesn't expose the correct semantics to the operating system's accessibility APIs.

I've given a more detailed explanation in at #2655794-14: Remove redundant WAI-ARIA role attributes from <main>, <nav>, <aside>, <header>, and <footer> elements and #2939485-2: Redundant attribute role="navigation" in Stable/Classy/Seven theme's pager.html.twig. Effectively, the explicit role="article" attribute serves as a polyfill for section 4.4 HTML Element Role Mappings in the HTML Accessibility API Mappings.

Comments #17 and #19 are incorrect. NO version of Internet Explorer has correctly mapped the implicit ARIA role for the <article> element (or any of the other elements mentioned in #24). Note that this is a separate issue from the use of a HTML5-shim, which was about getting IE to recognise the new HTML5 elements as being elements at all. Perhaps comments #17 and #19 have conflated it with that?

@alexpott - It would be a good idea for committers to request an accessibility review before removing a role attribute. There are lots of potential gotchas with removing a role attribute, which won't be apparent from reading the HTML recommendation on it's own. In this case, no comment has mentioned assistive tech support, which is the whole point of ARIA roles.

footer role="contentinfo"

Voiceover does not assign correct role to footer without role attribte.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/footer#Accessi...

@andrewmacpherson sorry for not tagging this one. OTOH at least I didn't commit it :) Should this be marked as a duplicate of #2655794: Remove redundant WAI-ARIA role attributes from <main>, <nav>, <aside>, <header>, and <footer> elements and included in the scope of that issue?

#26: Thanks for adding that WebKit issue. Indeed, IE11 isn't the only browser that has accessibility bugs. We certainly shouldn't remove these while we still support IE11, but even when we drop support for IE11, these role attributes will still need to be reassessed for the other browsers. Apple seem to have a lot of trouble keeping Safari and VoiceOver stable, particularly on iOS. Each new release of iOS has a different batch of accessibility bugs. I haven't kept a close watch on them all, but AppleVis has a good collection of them. That particular contentinfo bug is interesting, because it isn't clear whether the fault lies with WebKit, VoiceOver, or Apple's AX API.

#27: I'm not sure if it should be marked as a duplicate of #2655794: Remove redundant WAI-ARIA role attributes from <main>, <nav>, <aside>, <header>, and <footer> elements, for a couple of reasons. Firstly, the roles over there are all landmark roles, but article isn't a landmark role. Not many screen reader do anything with the article role (just JAWS and Talkback so far), so the stakes are lower than for landmark roles. Secondly, for reasons such as #26, we may need to consider each role separately (not lumping them together to be removed at the same time) so the related issue may yet get split up.

For those of us wanting to remove role="article" from our deployment, is it possible to dig this out using a theme hook rather than a patch to core?

#30 yes, you can probably remove the role attribute in a theme preprocess hook, or using a Twig without filter

But It's harmless, and there isn't a good reason to remove it.

For those who wish to address this in a twig template file right in your theme, this can be easily changed using the attribute class in node.html.twig, like so:

<article{{ attributes.addClass(classes).removeAttribute('role') }}>

This was the solution that worked best for us.

@andrewmacpherson - while this is not much of an issue for most users, it's satisfying to move our theme into the future just a little quicker; it also helps with accessibility improvements (and reporting), since the role="article" attribute isn't needed and throws warnings. For tightening up the dom output, the attribute class is really a great helper.

Unpostponing (poning?) since Drupal 10 no longer supports IE11.

Removed role="article" from <article> tag. Attached patch for same. please review

After manually reviewing #39 Patch, role="article" Removed from tag.

Verified and tested patch #39 on Drupal 10.1.x-dev. Patch was applied successfully and looks good to me.
Testing steps:
1. install Drupal 10.1.x
2. Check files UserPictureTest.php & node. module And find [role="article"] attribute.
3. apply the patch.
4. Observe that the [role="article"] attribute removed from the both files.

Testing Result:
1. [role="article"] attribute was removed after applying the patch. Screenshots are attached for reference:
Can be moved to RTBC"

As mentioned in https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/ar...

Don't use role="article". Instead use the element.

So this change looks good.

Not sure about the change record. If someone has overridden the template would say they have taken the responsibility upon themselves to be accessible. Even if they're embedding this just makes their site better.

But could be wrong.

Random

Agreed with @smustgrace on the lack of need for a change record, this is just removing something from preprocess.

I think we're find to go ahead here now that IE11 is no longer supported. I did copy @andrewmcpherson's comment from here over to #2655794: Remove redundant WAI-ARIA role attributes from <main>, <nav>, <aside>, <header>, and <footer> elements since we might want to be a bit more cautious there.