Pager h4 causes accessibility flag on many pages

aria-labelledby is used for referencing an element that has the desired label text. In this case, we want to get rid of that <h4>, so the solution in #2 would be referencing a nonexistent element.

Change that to aria-label, don't bother using heading_id at all (you're not using heading anymore), and you should be fine.. If the label is for assisitive tech only, it doesn't need to be contained in another element, you can define it directly with aria-label

Be sure to make the change to all views pager templates except those in stable9. Even though it is a visually hidden element, changing it is technically altering markup that would break the BC policy, though it could be argued (ideally in a followup so you can get the benefits ASAP) that it's an accessibility bugfix and should be applied to Stable9.

To assistive tech, this solution works identically so it's not at all disruptive. It accomplishes the same identification, but without the header-order-contaminating <h4>

The aria-labelledby was referencing a visually hidden element, so switching to the simpler aria-label takes care of this without any tradeoffs. If the <h4> was visible, it would potentially be worth another approach, but in this case it's not needed.

This is not changed in Stable9, so any debates regarding regression vs. bugfix can happen in the Stable9 scoped followup issue #3333418: Fix pager h4 for accessibility on Stable9. The easy wins can happen here, and that issue can deal with the remaining ones that might require discussion.

This arguably makes the heading_id property unnecessary in pager templates, so I opened a followup to address that, which can be tackled once this issue lands: #3333449: [PP-1] Consider deprecating heading_id property from pagination templates

Committed 01de0a8 and pushed to 10.1.x. Cherry-picked to 10.0.x and 9.5.x since this is a non-disruptive bug fix. Thanks!

I think this broke 9.5.x: https://www.drupal.org/pift-ci-job/2576989

Reverted and queuing the patch against that branch.

Looks good to me. Tested on Simlytest.me and does what is expected. Moving to RTBC!

I Saw this on the Drupal 9.5.3 release notes yesterday, including the reverting commit.

I think the proposal here is rather brutal. It doesn't take account of well-attested user behaviours. I don't think we should remove the heading.

@nicxvan - Thanks for reporting the issue. I appreciate the work you've done to maintain the patch here.

I'd like to take issue with the appraisal so far. Comment #8 by @bnjmnm moved this to RTBC:

To assistive tech, this solution works identically so it's not at all disruptive.

Emphasis mine. This remark is concerned with the assistive technology software, rather than the software's human users. The proposal may not be disruptive to software, but it is likely to be disruptive for users.

Previously, the pagination could be found by screen reader users in a number of ways:

1. Peruse headings. Many screen readers provide tools for this, but no widely used web browsers do so.
2. Using the web browser's find-in-page tool. It can match the content of the heading element. (Aside, you don't need actually need a screen reader for this.)
3. Peruse landmark regions. Many screen readers provide tools for this, but no widely used web browsers do so.

After implementing the proposal here, two of these methods no longer work:

1. The heading is gone, so you can't find the pager by perusing headings.
2. Using the browser's find-in-page tool no longer works. It doesn't match the content of the aria-label attribute.
3. The only method still available is to peruse landmark regions.

Will this be disruptive for actual users? Let's look at some user data.

The WebAIM Screen Reader User Survey #9 asked respondents:

When trying to find information on a lengthy web page, which of the following are you most likely to do first?

1. Navigate by headings was by far the most popular method, with approximately two-thirds of users preferring it.
2. Using a find-in-page tool was the next most popular, with approx 15% of users.
3. Navigating by landmark regions was the least popular method, at under 4% of users.

The same question was asked in earlier WebAIM surveys:

• Screen reader user surveys #8, #7, #5, and #4 showed roughly similar results to survey #9. It's been very consistent for a decade.
• Curiously, the question was omitted from survey #6.
• In surveys #2 and #3, headings and find-in-page were also the most popular methods. These surveys didn't offer landmark regions as an answer.

But the proposal here puts the kibosh on the two most popular methods, and doubles down on the least popular method!

Concerning the report from the aXe tests:

The pager by default uses an h4, this creates the "Heading levels should only increase by one" error in many use cases.

• Deque's documentation for the Heading levels should only increase by one test describes it as a "Deque best practice" of "moderate" impact.
• Deque's commentary describes why a sensible heading outline is useful, and gives recommendations for writing effective heading text.
• What the commentary doesn't address is: If a heading level is skipped, will any bad things happen? It completely fails to say how the skipped level presents a problem.
• The commentary doesn't encourage the removal of headings as a way to address the skipped level. On the contrary, the article goes to a lot of effort to advocate for the use of headings.

It's also worth noting that the skipped heading level IS NOT a failure of any WCAG success criteria. For a detailed explanation, see David Swallow's article: Heading off confusion: When do headings fail WCAG?

So, what's the impact (for users) of skipping a heading level? It's actually not as bad as the aXe rule title implies.

Screen readers are quite forgiving of missing heading levels. The missing heading level isn't a complete disaster.

Screen reader users can interact with headings in various ways:

1. All the screen readers I've ever used provide tools to move to the next/previous headings regardless of the heading level. Indeed, when using a touchscreen on a mobile OS, this is typically the only mechanism for moving amongst headings. The skipped heading level has no impact here whatsoever; the only thing that matters is the order of headings.
2. Many screen readers also provide commands to move among headings of a particular level, when using a keyboard. Most desktop screen readers have this feature, as do iOS VoiceOver and Android Talkback when a Bluetooth keyboard is connected. In practice this is very useful for jumping to the major sections of the page, while ignoring the intervening lower levels. It isn't quite as much use for navigating among the lower nested levels of headings; sometimes you ask for the next H4, only to be told that there isn't one.
3. A few screen readers (e.g. NVDA) provide a hierarchical tool to navigate headings as a tree. The NVDA tool is forgiving of skipped levels. It will show a H2 and H4, and still let you access the H4. AFAIK, no screen reader for a mobile OS provides a hierarchical heading browser, so the skipped heading level has no practical impact.

I'd like to stress that accessibility is about helping actual users, not satisfying opinionated best-practice rules in automated tests. It certainly isn't about keeping developer's dashboards green. Has any screen reader user asked for the heading to be removed?

Should we continue to provide a heading for the pagination? Yes. It facilitates the two most popular methods of finding information, as reported by the WebAim screen reader user survey.

Should we continue to use a landmark region? Yes. A moderate number of landmarks are very useful for finding the major sections of the page, and/or small-but-important tools on the page. Pagination for the main content is a good candidate for a landmark region.

Whilst landmark regions aren't the first tool that all users reach for, a simple landmark structure is a great accompaniment to a detailed heading outline. The belt-and-braces approach of using landmarks and headings offers choice for users.

For these reasons, I'd say no to the current proposal to remove the heading.

Alternative proposal: keep the heading. Can we improve the heading level?

We could deal with the missing level by promoting it to h3, and aXe would stop complaining.

The devil is in the details though...

Headings assume ownership over ALL of the content which follows, until another heading of the same level (or higher) is encountered.

For example, consider the admin/content page, with the default configuration from Standard profile.

Currently it looks like the pagination (h4) is a sub-heading under the breadcrumb navigation (the preceding h2). This is clearly nonsense, and promoting the pagination heading to h3 wouldn't improve that at all. It would still be a misleading heading structure, even though we'd eliminated the missing level which aXe complained about.

We could promote the pagination to a h2 level. Then it would be a subheading of the main page heading (h1), and a sibling of the breadcrumb and primary tabs. Much better.

Yet, a h2 assumes that the pagination relates to the main content of the page. For a views block in a sidebar region, the h2 would be too important; the views block would have it's own h2. Or, the views block might have no heading at all. Either way, giving the pagination a h2 in a views block would produce a misleading outline, suggesting it related to the main content.

Managing heading levels with a template-driven publishing system is a pain-in-the-architecture. There is no single correct level for every use of the template.

A more ambitious approach would be to compute heading levels, for the context of the assembled page. I'm not aware of prior PHP work which does this, but there's an interesting article from Heydon Pickering about how to do it in a React Javascript application: Managing Heading Levels In Design Systems. I expect there are still devils lurking in the details there too. Heydon's use of aria-level could complicate our existing CSS, but a server-side PHP approach might avoid using aria-level if we computed the heading levels late in the page render process.

Thank you @andrewmacpherson for your comprehensive and insightful response. Your thoughts and experiences are greatly appreciated, and your point about the potential regressions with some assistive technology users seems to warrant a revert.

I completely agree with you that real users should always be our top priority, and their needs and experiences should always be at the forefront of our considerations. Automated tools and tests are certainly helpful, but they are ultimately secondary to the needs of the people who are using the system.

If we were to revisit this and bring back the heading, my instinct would be to go via JS post-load -- querySelectorAll for H1, H2, H3, H4 and their aria equivalents. Then iterate the array and each time you hit a pagination heading, modify the pagination header's level to be one less than the previous level in the array.

That's what I do in Editoria11y -- I walk the tree comparing levels and flag things as needed.

And then...add a field to Views for "Pagination heading level." Let people manually pick a level if they know more than the machine, and have the default be auto, which pulls the JS library. To avoid breaking CSS changes to existing sites, the upgrade script could set existing views to "4" and only new views/installs would use the "auto..."

Re. #34 - Aha... thanks lauriii. I saw in the D9.5.3 release notes it was reverted, but I didn't realize it was still there in the D10 branches.

Re. #35 - We still have the heading, barring a brief blip in D10.0.3

That JS logic could be useful to study. I think server-side logic to write the correct level in HTML would be preferable to JS though.

I don't really mind the white noise, or the Drupal core issues that result from it. It's nice to know people run automated tests.

aXe has some ways to turn off certain tests; the browser plugin has a button to disable the "best practice" category, and if you run it from the CLI you can exclude specific tests I think. What you can't do is flag a specific test result as a false flag.

Mind you, there are some accessibility testing dashboards out there which let you mark individual reports as resolved/wontfix, and they don't bug you about it later. SiteImprove was one, and I think perhaps Deque or TPGi offerings might have this too. They're commercial, and SiteImprove had too many zeros on the end of the price for my liking, but this is possible.

The thing is, the aXe report here ISN'T a false report. It really would be better if we didn't skip a heading level. It's just that the impact isn't actually very severe in practice, and the proposed cure was worse than the disease.

Also I'd be wary of adding js just to prevent an accessibility flag in axe tools.

Yeah, I think that would be folly, too. It would also make us subject to ridicule and suspicion in the wider accessibility community.

Aside: there have been some disturbing reports alleging that an accessibility overlay company interfered with the WAVE accessibility checker, and WAVE's developers noticed this. Some nice bedtime reading at #accessiBe Spoofs Automated Checkers.

There is absolutely no way we can allow JS in Drupal core just to silence a test suite.

Simple, elegant; I like the approach.

Dodging the update script question...

• I'd go for an on-the-nose name for the machine name for the variable. E.g.: "pagination-heading-level" and "heading level." And probably best to go with heading rather than header, unless there is local precedent. I tend to get these confused myself...
• For the description; perhaps make it clear what people should do? E.g.: "Choose a heading level equal to or one lower than the section heading for your view."
• Miiiight also make sense to change the default level for *new* views to H3. That is more likely be correct out of the box, and avoids breaking themes. The update script could apply H4 to all existing views.
• Another approach that would dodge breaking existing sites would be leaving the H4 in code, and just modifying an attribute to override its level in the page outline: (aria-level="pagination-heading-level")
• You can leave the H1 off the option list in my opinion. A pager is going to be descendant of something, so it shouldn't ever need to be an H1.

Choose a heading level equal to or one lower than the section heading for your view.

This text made me realise that the functionality presumes the site admin can predict the context that the view will be rendered in. In some cases (eg a <drupal-view> embedded in content) this won't be the case. I'm not pitching the dynamic approach here, just that assigning a heading level at view config makes assumptions about the content structure the view will be used in later which can't be relied on. For the case of an embed, there are at least two cases:

• The view appears several times (perhaps with different parameters) in a document which contains headings at several levels
• The view appears in a content section which gets edited over time, so headings may change

Since views are configuration and commonly re-used, I expect there are other situations where the same might apply.

Instead of fixing to headings here, should we permit views to use headings or <nav>, allowing site admins to determine an appropriate method? That seems like it would fit within the changes to permit tag selection, if it's an acceptable change for accessibility requirements.

For the proposed description there's also the case where views have section headings disabled. If we stick with headings, I propose a less specific text for that description, eg "Choose a heading level appropriate to the context of the view placement". (It's not great, but it doesn't specify too much?)

@nicxvan, I don't see the new branch mentioned in #42 here - did that get missed? The only MR branch showing on the issue (for me) is the previously merged one.

Taking the opportunity in this comment to accept your offer in #3333401-28: Pager h4 causes accessibility flag on many pages for credit from #3232222: Pager template h4 can be out of document hierarchy order previously. (Hmm ... that checkbox isn't "sticking" for me when I select it.)

Tagging for WCAG SC 2.4.6 https://www.w3.org/WAI/WCAG21/Understanding/headings-and-labels.html

The attached patch expands on the work done in the issue fork - injecting the 'pagination_heading_level' into both full and mini pagers, as well as updating all core theme pager templates. For the life of me, I could not access 'pager_rewrite_elements' from the view config, so instead I added a method to filter out the heading elements from 'field_rewrite_elements'.

Blank line was missing necessary whitespace.

I think I may have a crlf issue

Submitted patch works, but fails a test -- Drupal\Tests\datetime\Functional\Views\FilterDateTest, due to a schema error despite numerous attempts to patch (views.pager.schema.yml).

Moving to NW for test failure

Also imagine this will need test coverage.

Also tagging for CR for changes to twig files.

Re-rolled patch to add guards for non-view pagers.

With the schema changes will require a post_update hook in addition to a change record.

Great work on the update hook!

There's a change to a fixture that I don't believe needs to be there.

Also since this is introducing a new UI element could a screenshot be added the User interface section.

Again great work!

Before applying the MR I did an export of the config to compare the views. Obviously didn't have the header config changes.

Applied the MR and the update hooks ran without issue
Did another config export and all my views have the header settings now.

Using the content view for manual testing I see the default H4
Changed it in the config to H2 and that worked like a charm.

Running test-only feature shows a few failures so think test coverage is there https://git.drupalcode.org/issue/drupal-3333401/-/jobs/376849

Believe this one is good!

Great work!

Added issue credits

@andrewmacpherson 💙💙💙 thank you so much for your thorough explanation, including data to support it. This community never ceases to amaze me.

Left some comments on the MR, great work here folks!

Appears feedback from @larowlan has been addressed.

Issue credits

Rebased this, will commit if passes

Thanks for all the changes @nicxvan

Committed to 11.x after rebase

I added an extra unit-test case for an invalid value and waited for a green run

diff --git a/core/modules/system/tests/src/Unit/Pager/PreprocessPagerTest.php b/core/modules/system/tests/src/Unit/Pager/PreprocessPagerTest.php
index b0fa383d800..24a12a53b47 100644
--- a/core/modules/system/tests/src/Unit/Pager/PreprocessPagerTest.php
+++ b/core/modules/system/tests/src/Unit/Pager/PreprocessPagerTest.php
@@ -158,4 +158,26 @@ public function testPaginationHeadingLevelSet() {
     $this->assertEquals('h5', $variables['pagination_heading_level']);
   }
 
+  /**
+   * Test template_preprocess_pager() with an invalid #pagination_heading_level.
+   *
+   * @covers ::template_preprocess_pager
+   */
+  public function testPaginationHeadingLevelInvalid() {
+    require_once $this->root . '/core/includes/theme.inc';
+    $variables = [
+      'pager' => [
+        '#element' => '2',
+        '#pagination_heading_level' => 'not-a-heading-element',
+        '#parameters' => [],
+        '#quantity' => '2',
+        '#route_name' => '',
+        '#tags' => '',
+      ],
+    ];
+    template_preprocess_pager($variables);
+
+    $this->assertEquals('h4', $variables['pagination_heading_level']);
+  }
+

Published the change record

Because this adds an update path, it can only be added in a minor so isn't eligible for backport.

Thanks again all.

Came here looking for explanation of why an additional option was added to Drupal, instead of the initial approach discussed on this issue of moving the heading text into an aria-label attribute on the parent tag, and thereby removing the Heading element altogether.

Had to dig for the answer buried in comment #30 and learned something in the process.

I've updated the issue summary with a sidenote about this point to help others not have to dig for the gold!

Thank you @andrewmacpherson; always enlightening.

.