In theme_item_list(), an empty h3 element could be generated, creating an accessibility problem. Most cases of this have been removed from D8; the first attached patch removes the remaining possibilities, such as if $variables['title'] was set to FALSE. The call to isset() is removed because the variable is always set near the beginning of the function. The (string) cast ensures that types like FALSE and NULL, which cast to empty string, will not trigger the h3 element to be generated. A call to !empty() is not used because a title of zero should be possible, for example.

The second patch fixes the problem for D7, where it is a bigger issue since there is no empty string check.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, tidy_list_title-D7.patch, failed testing.

Liam Morland’s picture

Status: Needs work » Needs review
FileSize
869 bytes

D8 patch by itself.

Everett Zufelt’s picture

Status: Needs review » Reviewed & tested by the community

This will ensure that only a non-empty string triggers the rendering of the h3.

xjm’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
580 bytes

I think this would accomplish the same thing, without the need for the typecast?

xjm’s picture

Oops, missed the bit in the summary. New patch inc. Actually, #2 is the least ugly way to do this, I think.

xjm’s picture

Status: Needs review » Reviewed & tested by the community

RTBC for #2 (not #4).

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

Committed/pushed #2, looks like this is backportable?

tim.plunkett’s picture

Status: Patch (to be ported) » Needs review
FileSize
783 bytes

This is only really useful in D8 in light of #256827: Various bugs in theme_item_list(), since that made the <div class="item-list"></div> only print when it had content.
Backporting just this is fine, but its not really as important.

xjm’s picture

Hmmm. I am not sure about this being backportable... it seems like a theme change, albeit a minor one. Someone might have code written around the current behavior.

Everett Zufelt’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs review » Fixed
Issue tags: -Needs backport to D7

Agreed with @xjm, and @tim.plunkett the benefit is minimal and although it is an edge case, it may break design on existing sites.

Status: Fixed » Closed (fixed)

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