Problematic hreflang links when using entity as front page.

I tested the patch ant it works great for me.

Actually, I noticed one problem. It's spitting out https:// for me, rather than http:// which is my current request protocol. My site has no SSL on it, though a shared SSL certificate is available. I have written a patch that can be applied after hreflang_front_page-2796399-2.patch (from #2) to adjust the logic. I have not tested this on an https site, but it works for my http site, in any case. Maybe someone can test it on SSL?

If this works, we can reroll so it can be applied cleanly with a single patch.

My patch failed testing but it is no surprise since mine was a follow to the initial patch. We'll need to re-roll later.

I have uploaded the patch by improving the coding standard issues. I hope it's correct

Re-rolling #10 which is the last working patch. I've made the adjustments that were intended in #12 and #14 and done a bit of cleanup.
With regard to the adjustments made in #10 for determining the protocol, I've removed that. Based on my local tests that appears to have been taken care of elsewhere in core but unfortunately I can't say exactly where. This patch applies to 8.7.x and 8.6.x.

Also, this related issue presents a wider perspective and could lead to a different solution to the problem: https://www.drupal.org/project/drupal/issues/2994800

Patch #19 worked for me. AFAIC this is RTBC.
But given the related issue, I wonder if I should change the status or not.

Patch #19 worked for me on 8.8.x.
botanic_spark I would go with this and merge it?

I added a test and updated the issue summary.

Needs review.

Needs a re-roll.

Working on it, Assigning this to me.

Rerolled #27 patch, Kindly review.

Thanks for the update. Tiny nitpick:

+++ b/core/modules/content_translation/content_translation.module
@@ -682,10 +683,21 @@ function content_translation_page_attachments(&$page) {
+          // If the current page is front page, do not create hreflang links
+          // from the entity route, just add the available languages to root path.

Exceeds 80 chars. Consider rewording so it doesn't spill to 3 lines, e.g.

// If the current page is the front page, do not create hreflang links
// from the entity route, just add the languages to the root path.

(You should confirm that fits within the 80 chars because a "the" was added to the first line.)

Thanks @Kristen Pol

here us the updated patch along with interdiff.

The interdiff looks good, thanks. Haven't had a chance to manually test it yet.

The patch from #34 works great for me.

Manually tested with and without patch and it works as expected.

Without patch

With patch but without front page set to node/2

With patch but with front page set to node/2

I just read all the comments above and it wasn't clear to me that anyone actually reviewed the code. Seems like people were just testing that it worked. I only did a superficial review before so I'll see if someone on the bugsmash team can do a better code review.

Changes and Test look good to me. Patch works as intended.

This is something SEO tools give a warning about, so it would be nice to get this in.

the patch looks good but there are a couple of issues:

FILE: ...upal/core/modules/content_translation/content_translation.module
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 707 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 14
 709 | ERROR | [x] Object operator not indented correctly; expected
     |       |     14 spaces but found 12
---------------------------------------------

This needs to be added to the cspell dictionary:

ontent_translation/tests/src/Functional/ContentTranslationLinkTagTest.php:130:14 - Unknown word (hreflangs)

I have made the changes as suggested above and updated the patch.
please review, thanks!

Hi thanks @sarvjeetsingh for patch.

I applied the patch on 9.1.x.
Before patch:

After applying the patch:

The points said by @catch are covered on the latest patch.

Committed 11118a3 and pushed to 9.1.x. Thanks!

Here is a reroll of the patch for 8.9.x, so that it applies for 8.9.6.

#46 reroll works fine for me for 8.9.6