Its possible for 2 regions to have the same alternate url:

<link rel="alternate" hreflang="en-de" href="https://site.com/germany-austria/en/">
<link rel="alternate" hreflang="en-at" href="https://site.com/germany-austria/en/">

The current implementation in HtmlResponseAttachmentsProcessor::processHtmlHeadLink generates a key that is used for rendering which uses only the href as unique key.

Because of this the second alternate url overwrites the first one and is not rendered.

CommentFileSizeAuthor
#23 core-hreflang_multiple_same_links_for_xdefault-2945033-23.patch3.04 KBvdsh
#21 interdiff.2945033.12-21.txt761 byteslongwave
#21 2945033-21.patch3.04 KBlongwave
#12 interdiff.2945033.9-12.txt3.06 KBlongwave
#12 2945033-12.patch3.06 KBlongwave
#12 2945033-12-test-only.patch2.01 KBlongwave
#9 html_head_link_duplicated_alternate_hreflang_2945033-9.patch1.1 KBjlatorre
#4 html_head_link_duplicated_alternate_hreflang_2945033_4.patch1.04 KByobottehg
#2 html_head_link_duplicated_alternate_hreflang_2945033.patch1 KByobottehg

Issue fork drupal-2945033

Command icon 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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yobottehg created an issue. See original summary.

yobottehg’s picture

yobottehg CreditAttribution: yobottehg commented
Status: Needs work » Needs review
FileSize
html_head_link_duplicated_alternate_hreflang_2945033.patch1 KB

Attached is a patch that specialcases alternate urls.

Status: Needs review » Needs work

The last submitted patch, 2: html_head_link_duplicated_alternate_hreflang_2945033.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

yobottehg’s picture

yobottehg CreditAttribution: yobottehg commented
Status: Needs work » Needs review
FileSize
html_head_link_duplicated_alternate_hreflang_2945033_4.patch1.04 KB

more specific about the used parameters

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

raphaeltbm’s picture

raphaeltbm CreditAttribution: raphaeltbm commented
Related issues: +#2994800: Improve the alternate hreflang implementation

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jlatorre’s picture

jlatorre CreditAttribution: jlatorre commented
FileSize
html_head_link_duplicated_alternate_hreflang_2945033-9.patch1.1 KB

Update patch to Drupal 8.7.x branch

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Elendev’s picture

Elendev CreditAttribution: Elendev at Swissquote commented

Hi @yobottehg or @jlatorre, I'm encountering the same issue. Do you need some help to test your patch?

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
2945033-12-test-only.patch2.01 KB
2945033-12.patch3.06 KB
interdiff.2945033.9-12.txt3.06 KB

The last submitted patch, 12: 2945033-12-test-only.patch, failed testing. View results

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Mirroar’s picture

Mirroar CreditAttribution: Mirroar at werk21 commented
Status: Needs review » Reviewed & tested by the community

I can confirm that #12 works for me and fixes the problem. Tests have passed as well, it seems, so I'd like to say RTBC.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/lib/Drupal/Core/Render/HtmlResponseAttachmentsProcessor.php
@@ -439,7 +439,15 @@ protected function processHtmlHeadLink(array $html_head_link) {
       $href = $attributes['href'];
-      $attached['html_head'][] = [$element, 'html_head_link:' . $attributes['rel'] . ':' . $href];
+      $rel = $attributes['rel'];
+
+      // Allow multiple hreflang tags to use the same href.
+      if ($rel === 'alternate' && isset($attributes['hreflang'])) {
+        $attached['html_head'][] = [$element, 'html_head_link:' . $rel . ':' . $attributes['hreflang'] . ':' . $href];
+      }
+      else {
+        $attached['html_head'][] = [$element, 'html_head_link:' . $rel . ':' . $href];
+      }

Should we be allowing hreflang even without rel = 'alternate'? I think this is legal even if it's rarely used?

DanielVeza’s picture

DanielVeza
he/him
Location Brisbane, AU
CreditAttribution: DanielVeza at Sparks Interactive commented
Status: Needs review » Reviewed & tested by the community

Applied #12 to a site where I had the bug where I could not set the x-default and the en-us hreflang links to be the same.

Worked correctly and saw the right output after the patch.

Should we be allowing hreflang even without rel = 'alternate'? I think this is legal even if it's rarely used?

I did a bit of searching around and couldn't find any examples of hreflang used without rel=alternate in the <head>. Considering that and the fact tests are passing I'm happy to mark this RTBC.

Happy to be overridden though!

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

The current HTML working spec seems to indicate that hreflang is valid for any relation type.

https://html.spec.whatwg.org/multipage/semantics.html#providing-users-wi... doesn't specify the relationships that hreflang is valid for, and just says the link element "could include" hreflang for any link.

rel="help" is not widely used but is included in the spec, I could imagine there may be context sensitive help available in different languages, for example.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work

Yeah I think we should allow what's valid according to the spec here rather than limit it to the one common usage.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
FileSize
2945033-21.patch3.04 KB
interdiff.2945033.12-21.txt761 bytes

Addressed #17-20 by removing the explicit check for rel="alternate".

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

vdsh’s picture

vdsh CreditAttribution: vdsh as a volunteer commented
FileSize
core-hreflang_multiple_same_links_for_xdefault-2945033-23.patch3.04 KB

Patch #21 rerolled for 9.2.3

Status: Needs review » Needs work

The last submitted patch, 23: core-hreflang_multiple_same_links_for_xdefault-2945033-23.patch, failed testing. View results

TMWagner made their first commit to this issue’s fork.

TMWagner opened merge request !1161

vdsh’s picture

vdsh CreditAttribution: vdsh as a volunteer commented
Status: Needs work » Needs review
alexverb’s picture

alexverb CreditAttribution: alexverb at iO commented
Status: Needs review » Reviewed & tested by the community

Solves the issue (for me in relation to: https://www.drupal.org/project/hreflang/issues/3126993). Putting on RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Version: 9.3.x-dev » 9.2.x-dev
Category: Feature request » Bug report
Status: Reviewed & tested by the community » Fixed

Committed and pushed 8f497e0a43 to 9.3.x and c1cc0d5c5d to 9.2.x. Thanks!

This is a bug not a feature :) - backported to 9.2.x because it is a low-risk bug fix.

  • alexpott committed 8f497e0 on 9.3.x 
    Issue #2945033 by longwave, yobottehg, TMWagner, vdsh, jlatorre, catch:...

  • alexpott committed c1cc0d5 on 9.2.x 
    Issue #2945033 by longwave, yobottehg, TMWagner, vdsh, jlatorre, catch:...
mfb’s picture

mfb
they or he
CreditAttribution: mfb commented

Not directly related to this issue, but I was looking at this code and opened #3245895: Link HTTP header should not be HTML-encoded - I don't think it's correct that core has been HTML-encoding the URL in the Link HTTP header.

Status: Fixed » Closed (fixed)

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