Attribute hreflang not allowed on element li at this point

li element shouldn't have the data-drupal-link-system-path either. Not sure why we want the li to have the active class. It doesn't happen for menus anywhere else.

No longer applies.

Not sure whether this helps in any way, but running the patch now leads to the following error (or, at least, it does for me):

Checking patch core/modules/system/src/Tests/Theme/FunctionsTest.php...
error: while searching for:
    $expected_links .= '<li class="a-link"><a href="' . Url::fromUri('base:a/link')->toString() . '">' . Html::escape('A <link>') .                                              '</a></li>';
    $expected_links .= '<li class="plain-text"><span class="a/class">' . Html::escape('Plain "text"') . '</span></li>';
    $expected_links .= '<li class="html-text"><span class="unescaped">' . Html::escape('potentially unsafe text that <should> be es                                             caped') . '</span></li>';
    $expected_links .= '<li data-drupal-link-system-path="&lt;front&gt;" class="front-page"><a href="' . Url::fromRoute('<front>')-                                             >toString() . '" data-drupal-link-system-path="&lt;front&gt;">' . Html::escape('Front page') . '</a></li>';
    $expected_links .= '<li data-drupal-link-system-path="router_test/test1" class="router-test"><a href="' . \Drupal::urlGenerator                                             ()->generate('router_test.1') . '" data-drupal-link-system-path="router_test/test1">' . Html::escape('Test route') . '</a></li>';
    $query = array('key' => 'value');
    $encoded_query = Html::escape(Json::encode($query));
    $expected_links .= '<li data-drupal-link-query="'.$encoded_query.'" data-drupal-link-system-path="router_test/test1" class="que                                             ry-test"><a href="' . \Drupal::urlGenerator()->generate('router_test.1', $query) . '" data-drupal-link-query="'.$encoded_query.'" d                                             ata-drupal-link-system-path="router_test/test1">' . Html::escape('Query test route') . '</a></li>';
    $expected_links .= '</ul>';
    $expected = $expected_heading . $expected_links;
    $this->assertThemeOutput('links', $variables, $expected);

error: patch failed: core/modules/system/src/Tests/Theme/FunctionsTest.php:279
error: core/modules/system/src/Tests/Theme/FunctionsTest.php: patch does not apply

Patch rerolled

Changes to pass tests

This patch (#26) is working for me. I've no longer got the hreflang attribute within the list element:

<ul class="links"><li class="en"><a href="/en" class="language-link" hreflang="en" data-drupal-link-system-path="&lt;front&gt;">English</a></li></ul>

Cool, we'll soon be able to check on http://validator.w3.org/check without falling ill :-)

I did validate it with https://validator.w3.org/ (Doctype HTML5(experimental)). Not sure whether we would have to check it any further.

So weirdly this is contradicting there own examples see https://www.w3.org/MarkUp/xhtml2/wiki/XHTML2/i18n/hreflang_examples

Are we sure we haven't discovered a bug in the validator?

Hum, you may be right.
But at first, everyone would think "the W3C must be right"

Not necessarily, the link describes UL nested in a P, not UL's nested in NAV elements, which is the case in this issue. Though it must be said that W3C also gives an error with the examples presented on the website.

The example above is for xhtml not html5. I still think the comment in #2454289-3: Attribute hreflang not allowed on element li at this point holds true as there is no link to anything.

So what do we do now ?

I guess #34 and #3 are right, and it could be also a bug in the validator.
Nonetheless it costs nothing to us to remove this attribute !

https://validator.w3.org/nu, another w3c tool, returns the same error as http://validator.w3.org/check. Both state that the HTML5 validator is in an experimental stage, so it wouldn't suprise me if #34 and #3 are correct. However, I can't find any issues on the respective github pages pertaining to this hreflang issue.

in fact, Drupal seems to use the attribute hreflang on the "li" to add an "is-active" class on it in the language switcher block (without it, all the "li" in the language switcher are set "is-active")

if only it was named "data-hreflang" instead it would be W3C valid

When I tried to apply this patch, I got the following error message: "Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/attribute_hreflang_not-2454289-26.patch".
So I recreated the patch for the version of Drupal I am using (8.3.7).

This is the new patch targeted against 8.5.x-dev.

This is the new patch targeted against 8.5.x-dev: hreflang_attribute_not-2454289-45.patch. Needs review.

@ajzetat : Something got wrong with your patch updad ?

One small nitpick.

+++ b/core/modules/language/tests/src/Functional/LanguageSwitchingTest.php
@@ -93,8 +92,8 @@ protected function doTestLanguageBlockAuthenticated($block_label) {
+      0 => array('langcode_class' => 'en'),
+      1 => array('langcode_class' => 'fr'),

This should use the short notation for arrays ([] instead of array())

Applied suggestion at #49 by borisson_.

#50 applied to core 8.53 and fixed the issue according to https://validator.w3.org/nu/

This patch applies to 8.5.6 without issues and solves the problem and validation now passes. Would be nice to get this in.

The patch also applies to 8.6 with proper validation. However, I'm wondering whether we won't be breaking any functionality in the drupal.active-link library. Are we sure this is the way to go?

Patch for use with 8.6.x

Patches should be applicable to 8.7.x now, so I've added a test for that.

Here is an alternative approach that wont affect the drupal.active-link library, (but probably breaks the active link library for all other links where the hreflang was on an actual a tag)

@Rob Holmes: If I understand correctly, patch #60 replaces the hreflang attribute with a custom HTML5 data attribute. I think there is a rule that states that data attributes should not be used if there is in fact already an existing attribute which is more appropriate for storing the data (language in this case). Hreflang is the more appropriate attribute even if it isn't allowed on a li element. Therefore, while applying this patch might resolve the w3c validation error, it would introduce another non-detectable 'error'.

That's a fair assessment,

Maybe we need a test for the active link library to ensure that nothing gets broken by any changes introduced by removing the hreflang from li elements?

That might be useful. The LanguageSwitchingTest seems to check some things with the active-link library, so I would say we're at least partway there. I've noticed that the library itself has a redundant variable initializer, but I'm not touching that since it has a big "DO NOT EDIT THIS FILE" warning in it.

The patch in #58 is functionally the same as the patch in #50, so I have no issues with that. However, issue #2713451 introduced a long comment stating why the 'is-active' class ha been placed on the li element. Apparently, it is needed to allow different styling on an active element. I'll ask the reporter of that issue to take a look at this issue.

> Apparently, it is needed to allow different styling on an active element.

That's pretty much the reason the 'is-active' class should be on the LI, and not just the A element. Sometimes you want to apply some sort of highlighting to the whole LI element.

Hello, I update this solution, because when we get originalSelectors in this array get all the items, not only li. And as a result we get a bug through which the two elements are placed in the class "is-active".

Updated patch and fix problem with failing test.

Fixed issue for not authentication users.

I think for the original issue or question the solution is rather simple.
Please do tell me if I am forgetting something but in this patch I just removed the hreflang from the LI element.

@Mschudders while that technically fixes the validation error, it regresses the ability for Drupal.behaviors.activeLinks to highlight which language is the active one (which it does via javascript)

Any solution to this problem will have to provide valid HTML as well as retaining the ability to have a correctly applied .active class.

Okay but I still see the is-active classes:

From the screenshot you have attached it looks like you have an .is-active class on the NL language as well as the EN Language links?

@Mschudders, can you apply my patch #63? In this patch I fix bug when hreflang attributes missing in li elements. And ".is-active" sets correctly.

Changed the test ActiveLinkResponseFilterTest to validate the results.

Fixed error in the test.

NR for testbot.

Please credit eugene.brit and ksemihin since they helped Zulljin locally in the office.

RTBC.
Patch 82 resolves the related accessibility issue and the test looks correct and corresponds to the changes required for w3c validation.

It also applies cleanly to head of 8.7.x and works correctly for 8.7.x as well.

Thanks

Re #85 - what's the accessibility issue you refer to? It isn't mentioned anywhere earlier in the issue.

accessibility:
for the language switcher block in particular (our site is two languages English / French)
Language of Parts (Level AA)
1. All English pages: require (lang=”fr”) on Francais language toggle link
2. All French pages: require (lang=”en”) on English language toggle link

@joseph.olstad - The issue summary here is about a HTML validation error, where <li hreflang="es"> isn't allowed. That has nothing to do with WCAG Language of Parts.

You can't solve Language of Parts with the hreflang attribute. The attribute indicates the language of the linked resource, but WCAG Language of Parts is concerned with the language of the link text. There's a separate issue to address this on the language switcher block - #3049122: Links in language switcher block do not conform to WCAG language-of-parts. I started to look into a solution for that, and noticed the HTML validation error on the containing <li> element, which led me to this issue.

I tried patch #82 with the specimen content in the Umami demo, and the language switcher block:

• It does solve the HTML validation error from <li hreflang="es">.
• It leaves the <a hreflang="es"> attribute in place on the link.
• It doesn't solve the WCAG Language of Parts problem with the language switcher block. Specifically, it doesn't introduce any lang attributes. That's OK, because the issue summary here doesn't claim to fix that.

Crediting eugene.brit and ksemihin per #84

A couple of things which might need work. I don't know if they're important though, so I'm leaving it at RTBC.

1. What's going on with the <li data-drupal-link-system-path> attribute? In #4, _nod suggested removing it from the list item. It's still there though. It looks like an earlier patch (#26) removed it, but by patch #82 it's come back.
2. The new data-drupal-language attribute isn't very well explained.

+++ b/core/lib/Drupal/Core/EventSubscriber/ActiveLinkResponseFilter.php
@@ -187,7 +187,10 @@ public static function setLinkActiveClass($html_markup, $current_path, $is_front
-        if ($node->hasAttribute('hreflang') && $node->getAttribute('hreflang') !== $url_language) {
+        if ($node->nodeName !== 'a' && $node->hasAttribute('data-drupal-language') && $node->getAttribute('data-drupal-language') !== $url_language) {
+          $add_active = FALSE;
+        }
+        elseif ($node->nodeName === 'a' && $node->hasAttribute('hreflang') && $node->getAttribute('hreflang') !== $url_language) {
           $add_active = FALSE;
         }

I think we can tidy this up a bit by doing.

        $attribute = $node->nodeName === 'a' ? 'hreflang' : 'data-drupal-language';
        if ($node->hasAttribute($attribute) && $node->getAttribute($attribute) !== $url_language) {
          $add_active = FALSE;
        }

Also #82 had changes to the core/misc/active-link.js that were not build from core/misc/active-link.es6.js so I've fixed that.

#95 not applied for drupal 9.0.

Here is 95 rerolled for 9.0.x
hope the reroll is ok.

reroll for 9.1.0

Patch in comment #95 appears to work well for Drupal 8.9.14

needs a reroll for 9.0 and probably 9.1/9.2/9.3

Patch #95 in Drupal 8.9.19 makes my default links language switcher block having no more hreflang tag attributes on the li.
BUT it makes all li having an is-active class.

Patch #101 in Drupal 9.2.5 works fine, no more hreflang tag attributes on the li. And I do have the "is-active" class right on the li containing the displayed language link.

I just tried patch 101 and 109 and I am getting the following error.

My settings is:
Drupal Version
9.4.5

PHP
Version
7.4.30

Database MySQL
Version
5.7.39

Checking patch core/includes/theme.inc...
error: core/includes/theme.inc: No such file or directory
Checking patch core/lib/Drupal/Core/EventSubscriber/ActiveLinkResponseFilter.php...
error: core/lib/Drupal/Core/EventSubscriber/ActiveLinkResponseFilter.php: No such file or directory
Checking patch core/misc/active-link.es6.js...
error: core/misc/active-link.es6.js: No such file or directory
Checking patch core/misc/active-link.js...
error: core/misc/active-link.js: No such file or directory
Checking patch core/tests/Drupal/Tests/Core/EventSubscriber/ActiveLinkResponseFilterTest.php...
error: core/tests/Drupal/Tests/Core/EventSubscriber/ActiveLinkResponseFilterTest.php: No such file or directory

Note I am testing in a local environment and then upload to a shared server

Patch #109 applies on 9.5.x, but needs a reroll for 10.1.x as core/misc/active-link.es6.js was renamed to core/misc/active-link.js

Rerolled 109:
This new patch works with D9.4.9 and D9.5.0

straight up reroll, I changed nothing except fix conflict rejects on hreflang substitute with data-drupal-language in
core/tests/Drupal/Tests/Core/EventSubscriber/ActiveLinkResponseFilterTest.php

ok the CI framework is really picky, patch 113 is clean for D9.4.9 however it will work for D9.5.0 also

however to please the very picky CI framework that doesn't accept fuzz, I've got another D9.5.0 patch, 114

Re-roll #113 for 10.0.x

Re-roll #116 against 10.1.x and 11.x. The custom commands failure from #116 should have been fixed.

I've re-read through every comment in this issue and have observed the following:

data-drupal-link-system-path

in #4, @nod said

li element shouldn't have the data-drupal-link-system-path either.

in #94, @andrewmacpherson said

What's going on with the <li data-drupal-link-system-path> attribute? In #4, _nod suggested removing it from the list item. It's still there though. It looks like an earlier patch (#26) removed it, but by patch #82 it's come back.

This hasn't been addressed, so we need to decide whether this is something that needs to be done here or not, and update the issue summary and description accordingly.

data-drupal-language

in #94 @andrewmacpherson also said

The new data-drupal-language attribute isn't very well explained.

and this hasn't been addressed yet either.

failing tests in #118 need addressing

needs re-rolling

Here's the re-rolls for 10.3.x and 11.x - both based on #118

The other items from #121 remain outstanding.

Created a merge request and adjusted the unit test.

I think changing data-drupal-link-system-path is out of scope. This was added on purpose in #1979468: ".active" from linkGenerator(), l() and theme_links() forces an upper limit of per-page caching for all content containing links and the concept of having the active class on li links has existed even before that. It's not broken and doesn't cause problems.

Not sure what/where to document on data-drupal-language, it's an internal attribute, like the other two, I don't think they are documented either.

Have not reviewed yet but issue summary could use some love

If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

Updated the issue summary. Didn't mention the discussion around the other data attributes, I don't really see how that's in scope. This doesn't change them in any way and we afaik specifically support active classes on list items.

Hiding patches as the fix is in the MR,

Thanks for updating the issue summary.

Applying the MR and enabling the translation modules I am seeing the change being made see screenshots.

Before

After

Don't see any open threads or pending items right now. And nothing seems off in the code.

Rebased this, had to adjust script size. The diff is a bit misleading because other changes were already causing it to be very close to the upper limit of the 500 +/- range, the actual value before was 170941, now it is 171197 (which I rounded up to 171200), so a difference of 256.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Hello, here is a patch for version 10.4.x that takes the same changes as the one made in 11.x

Thanks but the issue has an MR already open and should be continued there

Rerolled MR on latest 11.x

Test failure seems related to change.

Any BC concern?

Looks like other changes might have adjusted it again so it's close enough to the threshold, lets see if this passes.

This is a bugfix that removes invalid markup. It's a change, but it's a required one. You RTBC'd it 7 months ago, it just had a conflict and I missed it.

Snapshot of latest state of MR for safe usage with composer patches for D11.2 projects

Using the command git diff bf76a69d I verified that the code is the same as that which was RTBCed back in March 2025 in comment #128. Subsequent changes to adjust the ScriptBytes in performance tests have been reverted. I'm restoring the RTBC status.

We should add a change record to detail what is changing here and the new data-drupal-language attribute.

I've considered the possible API impacts of this change and note that we're allowed to make markup changes in minor releases so this okay to go in 11.x - it might have missed 11.3.x but I think it is okay to go in 11.4.0 / 12.0.0 - will discuss with release managers if the CR is created and this is back to RTBC before 11.3.0-beta time...

I took a shot at adding a CR. Please review https://www.drupal.org/node/3556699.

Looks good to me, I tried to explain a bit better what exactly "invalid" means.

Committed and pushed 7c721031d5d to 11.x and f0823b6cbb7 to 11.3.x. Thanks!

Decided to backport to 11.3.x as html changes are allowed and I've created contrib and I do not believe this will have an impact.

Cool, one less patch, thanks!