Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hreflang not correctly returned from GraphQL #1376

Closed
FreekVR opened this issue Nov 7, 2023 · 34 comments
Closed

Hreflang not correctly returned from GraphQL #1376

FreekVR opened this issue Nov 7, 2023 · 34 comments
Labels

Comments

@FreekVR
Copy link

FreekVR commented Nov 7, 2023

Describe the bug

We're seeing all the hreflangs being returned from GraphQL being equal to the english / unprefixed version of the pages, even though the canonical ánd url values are correct, even the hreflang pointing to the current language isn't correct.

To reproduce

Steps to reproduce the behaviour:

  1. In GraphiQL, enter the following query;
query MyQuery {
  entries(site: ["en", "nl"]) {
    url
    language
    seomatic(asArray: true) {
        metaLinkContainer
    }
  }
}

returns

{
      {
        "url": "http://localhost:3000/nl/locations",
        "language": "nl-NL",
        "seomatic": {
          "metaLinkContainer": "{\"canonical\":{\"href\":\"http:\\/\\/localhost:3000\\/nl\\/locations\",\"rel\":\"canonical\"},\"home\":{\"href\":\"http:\\/\\/localhost:3000\",\"rel\":\"home\"},\"author\":{\"href\":\"http:\\/\\/localhost:3000\\/humans.txt\",\"rel\":\"author\",\"type\":\"text\\/plain\"},\"publisher\":[],\"alternate\":[{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"pt-pt\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"es-es\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"fr-fr\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"de-de\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"nl-nl\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"x-default\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"en-gb\",\"rel\":\"alternate\"}]}"
        }
      },
      {
        "url": "http://localhost:3000/locations",
        "language": "en-GB",
        "seomatic": {
          "metaLinkContainer": "{\"canonical\":{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"rel\":\"canonical\"},\"home\":{\"href\":\"http:\\/\\/localhost:3000\",\"rel\":\"home\"},\"author\":{\"href\":\"http:\\/\\/localhost:3000\\/humans.txt\",\"rel\":\"author\",\"type\":\"text\\/plain\"},\"publisher\":[],\"alternate\":[{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"pt-pt\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"es-es\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"fr-fr\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"de-de\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"nl-nl\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"x-default\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/locations\",\"hreflang\":\"en-gb\",\"rel\":\"alternate\"}]}"
        }
      },
      {
        "url": "http://localhost:3000/nl/stories",
        "language": "nl-NL",
        "seomatic": {
          "metaLinkContainer": "{\"canonical\":{\"href\":\"http:\\/\\/localhost:3000\\/nl\\/stories\",\"rel\":\"canonical\"},\"home\":{\"href\":\"http:\\/\\/localhost:3000\",\"rel\":\"home\"},\"author\":{\"href\":\"http:\\/\\/localhost:3000\\/humans.txt\",\"rel\":\"author\",\"type\":\"text\\/plain\"},\"publisher\":[],\"alternate\":[{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"pt-pt\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"es-es\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"fr-fr\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"de-de\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"nl-nl\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"x-default\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"en-gb\",\"rel\":\"alternate\"}]}"
        }
      },
      {
        "url": "http://localhost:3000/stories",
        "language": "en-GB",
        "seomatic": {
          "metaLinkContainer": "{\"canonical\":{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"rel\":\"canonical\"},\"home\":{\"href\":\"http:\\/\\/localhost:3000\",\"rel\":\"home\"},\"author\":{\"href\":\"http:\\/\\/localhost:3000\\/humans.txt\",\"rel\":\"author\",\"type\":\"text\\/plain\"},\"publisher\":[],\"alternate\":[{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"pt-pt\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"es-es\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"fr-fr\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"de-de\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"nl-nl\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"x-default\",\"rel\":\"alternate\"},{\"href\":\"http:\\/\\/localhost:3000\\/stories\",\"hreflang\":\"en-gb\",\"rel\":\"alternate\"}]}"
        }
      },
}

Expected behaviour

The hreflang alternates match the URL or canonical of the given items' tranlations

Versions

  • Plugin version: 4.0.33
  • Craft version: 3.5.9
@FreekVR FreekVR added the bug label Nov 7, 2023
@khalwat
Copy link
Collaborator

khalwat commented Nov 10, 2023

I'll have to set up a similar environment here to test, but as a sanity check, are all of your Craft CMS Sites set up with fully qualified domain name, and not a relative path or such?

@FreekVR
Copy link
Author

FreekVR commented Nov 10, 2023

Sites don't have a dedicated domain, but they do have their domain set as follows:

project.yaml
baseUrl: '@frontend/nl'

config/general.php

        'aliases' => [
            '@frontend' => App::env('DEFAULT_SITE_URL')
        ],

.env

DEFAULT_SITE_URL="http://localhost:3000" # URL of the default site

I would assume if the issue was the site config however, that the craft url and Seomatic canonical for the entries on these sites would also be incorrect.

@khalwat
Copy link
Collaborator

khalwat commented Nov 10, 2023

Are you able to show me a screenshot of one of the sites that isn't working as expected, from the Craft CP: Settings -> Sites -> click on a site.

Here's an example:

Screenshot 2023-11-10 at 11 34 34

@FreekVR
Copy link
Author

FreekVR commented Nov 13, 2023

Sure, here's two.

The english (default site) appears to be working correctly -- in fact -- it's URLs are shown as the alternate for all languages right now.

image
image

@FreekVR
Copy link
Author

FreekVR commented Nov 13, 2023

I just did a quick check -- it IS using a translated slug if used but NOT the suffix from the Base URL? Again, this is applied correctly to/for the canonical, but not the alternates?

Here's an example (the NL links should be prefixed with /nl/)

{
  entries(site: ["nl", "en"], id: 53) {
    url
    uri
    seomatic {
      metaLinkContainer
    }
  }
}
{
  "data": {
    "entries": [
      {
        "url": "http://localhost:3000/test-en",
        "uri": "test-en",
        "seomatic": {
          "metaLinkContainer": "<link href=\"http://localhost:3000/test-en\" rel=\"canonical\">\n<link href=\"http://localhost:3000\" rel=\"home\">\n<link type=\"text/plain\" href=\"http://localhost:3000/humans.txt\" rel=\"author\">\n<link href=\"http://localhost:3000/test-nl\" rel=\"alternate\" hreflang=\"nl-nl\">\n<link href=\"http://localhost:3000/test-en\" rel=\"alternate\" hreflang=\"x-default\">\n<link href=\"http://localhost:3000/test-en\" rel=\"alternate\" hreflang=\"en-gb\">"
        }
      },
      {
        "url": "http://localhost:3000/nl/test-nl",
        "uri": "test-nl",
        "seomatic": {
          "metaLinkContainer": "<link href=\"http://localhost:3000/nl/test-nl\" rel=\"canonical\">\n<link href=\"http://localhost:3000\" rel=\"home\">\n<link type=\"text/plain\" href=\"http://localhost:3000/humans.txt\" rel=\"author\">\n<link href=\"http://localhost:3000/test-nl\" rel=\"alternate\" hreflang=\"nl-nl\">\n<link href=\"http://localhost:3000/test-en\" rel=\"alternate\" hreflang=\"x-default\">\n<link href=\"http://localhost:3000/test-en\" rel=\"alternate\" hreflang=\"en-gb\">"
        }
      }
    ]
  }
}

@khalwat
Copy link
Collaborator

khalwat commented Dec 6, 2023

So sorry for not getting back to you sooner on this... in my test environment, here's the query I'm doing:

query MyQuery {
  entries(site: ["default", "spanish"]) {
    url
    language
    seomatic(asArray: false) {
        metaLinkContainer
    }
  }
}

...and here's the raw result I'm seeing:

      {
        "url": "http://localhost:8004/blog/my-first-blog",
        "language": "en-US",
        "seomatic": {
          "metaLinkContainer": "<link href=\"http://localhost:8004/blog/my-first-blog\" rel=\"canonical\">\n<link href=\"http://localhost:8004\" rel=\"home\">\n<link type=\"text/plain\" href=\"http://localhost:8004/humans.txt\" rel=\"author\">\n<link href=\"http://localhost:8004/es/blog/my-first-blog\" rel=\"alternate\" hreflang=\"es\">\n<link href=\"http://localhost:8004/blog/my-first-blog\" rel=\"alternate\" hreflang=\"x-default\">\n<link href=\"http://localhost:8004/blog/my-first-blog\" rel=\"alternate\" hreflang=\"en-us\">"
        }
      },
      {
        "url": "http://localhost:8004/es/blog/my-first-blog",
        "language": "es",
        "seomatic": {
          "metaLinkContainer": "<link href=\"http://localhost:8004/es/blog/my-first-blog\" rel=\"canonical\">\n<link href=\"http://localhost:8004/es\" rel=\"home\">\n<link type=\"text/plain\" href=\"http://localhost:8004/es/humans.txt\" rel=\"author\">\n<link href=\"http://localhost:8004/es/blog/my-first-blog\" rel=\"alternate\" hreflang=\"es\">\n<link href=\"http://localhost:8004/blog/my-first-blog\" rel=\"alternate\" hreflang=\"x-default\">\n<link href=\"http://localhost:8004/blog/my-first-blog\" rel=\"alternate\" hreflang=\"en-us\">"
        }

Teasing this out, I am seeing the correct prefixes for the URLs:

english siteUrl: http://localhost:8004/

<link href="http://localhost:8004/blog/my-first-blog" rel="canonical">

<link href="http://localhost:8004/es/blog/my-first-blog" rel="alternate" hreflang="es">
<link href="http://localhost:8004/blog/my-first-blog" rel="alternate" hreflang="x-default">
<link href="http://localhost:8004/blog/my-first-blog" rel="alternate" hreflang="en-us">"

spanish siteUrl: http://localhost:8004/es/

<link href="http://localhost:8004/es/blog/my-first-blog" rel="canonical">

<link href="http://localhost:8004/es/blog/my-first-blog" rel="alternate" hreflang="es">
<link href="http://localhost:8004/blog/my-first-blog" rel="alternate" hreflang="x-default">
<link href="http://localhost:8004/blog/my-first-blog" rel="alternate" hreflang="en-us">"

In the case of the Spanish site, the URLs do have the correct language prefixes, so I'm thinking this must be something setup-related, because it's working as expected in my test environment here.

@khalwat
Copy link
Collaborator

khalwat commented Dec 6, 2023

Is it possible that your entries are not localized for the sites in question or such? I'm a little confused at how this could be happening at this point.

@jornwildenbeest
Copy link

Hi, not sure if this issue is related to mine, if not I will create a new issue.

But I'm have a same kind of problem with a headless website end the hreflang tag.

So for the record. I have a craft 4 multi lang setup with a headless frontend.
Because of the headless frontend I used the seomatic setting Site URL Override.

But with this setting enabled all my hreflang tags for all my languages use this url from the Url Override setting.

FYI: Correct frontend urls:
en: https://my-frontend-url.com
nl: https://my-frontend-url.com/nl
de: https://my-frontend-url.com/de

Current situation with Site URL Override enabled and set to my frontend url:

<link data-n-head="ssr" href="https://my-frontend-url.com/test-page" hreflang="en" rel="alternate">
<link data-n-head="ssr" href="https://my-frontend-url.com/test-page" hreflang="nl" rel="alternate">
<link data-n-head="ssr" href="https://my-frontend-url.com/test-page" hreflang="de" rel="alternate">

What you expect:

<link data-n-head="ssr" href="https://my-frontend-url.com/test-page" hreflang="en" rel="alternate">
<link data-n-head="ssr" href="https://my-frontend-url.com/nl/test-page" hreflang="nl" rel="alternate">
<link data-n-head="ssr" href="https://my-frontend-url.com/de/test-page" hreflang="de" rel="alternate">

With the Site Url Override setting disabled it works like it should, but with the backend url.. So that's no good.

@khalwat
Copy link
Collaborator

khalwat commented Dec 6, 2023

Oh, if you're using the Site URL Override setting, that only allows you to provide a single site URL to override, so I'm not surprised that it wouldn't work properly with multiple sites.

The only solution I can think of for this specific type of setup would be either:

  1. Make sure your Site URLs are set to what they should be for each frontend site (and that you have your cpSiteUrl set for the CP), and don't use the Site URL Override setting

  2. We make the config setting for Site URL Override be able to pass in an array of site URLs to override, indexed by the language handle or such

#1 would be a much preferred solution here though @jornwildenbeest

@jornwildenbeest
Copy link

@khalwat i understand that option 1 is the easiest solution.
But I'm afraid that is not an option for us because use the craft backend environment as a platform only for a certain amount of users. So we cannot simply change all urls to the frontend headless url.

I do think that it is a better and permanent solution to improve the nystudio107\seomatic\helpers\UrlHelper::siteUrl function in a way that you can use it with the Site Url Override option, but also include the language handle etc. I think that is what you mean with option 2.

@khalwat
Copy link
Collaborator

khalwat commented Dec 6, 2023

Yeah I mean we're getting super-niche here, the issue you're running into only happens if:

  1. You have a headless Craft site
  2. That Craft site is a multi-site in multiple languages
  3. Craft is only used for a subset of the pages on said site, so you can't properly set the Craft Site URLs

I'll have to see how this could potentially be implemented without causing any existing sites to break when changing how the Site URL Override setting is implemented.

khalwat added a commit that referenced this issue Dec 6, 2023
…either a string, or an array of site URLs, indexed by the site handle for overriding complex headless multi-site Craft setups ([#1376](#1376))
khalwat added a commit that referenced this issue Dec 6, 2023
…either a string, or an array of site URLs, indexed by the site handle for overriding complex headless multi-site Craft setups ([#1376](#1376))
@khalwat
Copy link
Collaborator

khalwat commented Dec 6, 2023

@jornwildenbeest @FreekVR Added via: 3d30331 & 90ed2cd

Please test this out, and ensure that it works as needed for your situation.

Craft CMS 3:

You can try it now by setting your server in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop as 3.4.68”,

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop-v4 as 4.0.37”,

Then do a composer clear-cache && composer update

@khalwat khalwat closed this as completed Dec 6, 2023
@jornwildenbeest
Copy link

Hi @khalwat,

Thanks for the quick response!

Looks promising, but with Craft 4.4.17 and php 8.1 I am getting the following error:

2023-12-07 06:25:46 [web.ERROR] [TypeError] TypeError: craft\helpers\UrlHelper::urlWithParams(): Argument #2 ($params) must be of type array|string, null given, called in /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/UrlHelper.php on line 58 and defined in /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/helpers/UrlHelper.php:101
Stack trace:
#0 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/UrlHelper.php(58): craft\helpers\UrlHelper::urlWithParams('https://my-bac...', NULL)
#1 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/DynamicMeta.php(359): nystudio107\seomatic\helpers\UrlHelper::siteUrl('', NULL, NULL, 2)
#2 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/DynamicMeta.php(285): nystudio107\seomatic\helpers\DynamicMeta::addMetaJsonLdBreadCrumbs(2)
#3 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/services/MetaContainers.php(422): nystudio107\seomatic\helpers\DynamicMeta::addDynamicMetaToContainers('', 2)
#4 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/services/MetaContainers.php(305): nystudio107\seomatic\services\MetaContainers->loadMetaContainers('', 2)
#5 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/Container.php(106): nystudio107\seomatic\services\MetaContainers->previewMetaContainers('', 2, true)
#6 [internal function]: nystudio107\seomatic\helpers\Container::nystudio107\seomatic\helpers\{closure}(Object(craft\cache\FileCache))
#7 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/caching/Cache.php(607): call_user_func(Object(Closure), Object(craft\cache\FileCache))
#8 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/Container.php(97): yii\caching\Cache->getOrSet('seomatic_metaco...', Object(Closure), 1, Object(yii\caching\TagDependency))
#9 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/gql/resolvers/SeomaticResolver.php(64): nystudio107\seomatic\helpers\Container::getContainerArrays(Array, '', 2, true)
#10 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(623): nystudio107\seomatic\gql\resolvers\SeomaticResolver::resolve(NULL, Array, Array, Object(GraphQL\Type\Definition\ResolveInfo))
#11 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(550): GraphQL\Executor\ReferenceExecutor->resolveFieldValueOrError(Object(GraphQL\Type\Definition\FieldDefinition), Object(GraphQL\Language\AST\FieldNode), 'nystudio107\\seo...', NULL, Object(GraphQL\Type\Definition\ResolveInfo))
#12 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(1195): GraphQL\Executor\ReferenceExecutor->resolveField(Object(GraphQL\Type\Definition\ObjectType), NULL, Object(ArrayObject), Array)
#13 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(264): GraphQL\Executor\ReferenceExecutor->executeFields(Object(GraphQL\Type\Definition\ObjectType), NULL, Array, Object(ArrayObject))
#14 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(215): GraphQL\Executor\ReferenceExecutor->executeOperation(Object(GraphQL\Language\AST\OperationDefinitionNode), NULL)
#15 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/Executor.php(156): GraphQL\Executor\ReferenceExecutor->doExecute()
#16 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/GraphQL.php(162): GraphQL\Executor\Executor::promiseToExecute(Object(GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter), Object(GraphQL\Type\Schema), Object(GraphQL\Language\AST\DocumentNode), NULL, Array, Array, NULL, NULL)
#17 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/GraphQL.php(94): GraphQL\GraphQL::promiseToExecute(Object(GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter), Object(GraphQL\Type\Schema), '{\n  seomatic(ur...', NULL, Array, Array, NULL, NULL, Array)
#18 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/services/Gql.php(512): GraphQL\GraphQL::executeQuery(Object(GraphQL\Type\Schema), '{\n  seomatic(ur...', NULL, Array, Array, NULL, NULL, Array)
#19 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/controllers/GraphqlController.php(181): craft\services\Gql->executeQuery(Object(craft\models\GqlSchema), '{\n  seomatic(ur...', Array, NULL, true)
#20 [internal function]: craft\controllers\GraphqlController->actionApi()
#21 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#22 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams(Array)
#23 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction('api', Array)
#24 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/web/Application.php(304): yii\base\Module->runAction('graphql/api', Array)
#25 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/web/Application.php(607): craft\web\Application->runAction('graphql/api', Array)
#26 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/web/Application.php(283): craft\web\Application->_processActionRequest(Object(craft\web\Request))
#27 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/Application.php(384): craft\web\Application->handleRequest(Object(craft\web\Request))
#28 /Users/jorn/Sites/my-backend-site/web/index.php(12): yii\base\Application->run()
#29 /Users/jorn/.composer/vendor/laravel/valet/server.php(250): require('/Users/jorn/Sit...')
#30 {main} {"memory":59509384,"exception":"[object] (TypeError(code: 0): craft\\helpers\\UrlHelper::urlWithParams(): Argument #2 ($params) must be of type array|string, null given, called in /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/UrlHelper.php on line 58 at /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/helpers/UrlHelper.php:101)"} 
2023-12-07 06:25:46 [web.INFO] [application] Request context:
$_GET = [
    'site' => 'default'

@FreekVR
Copy link
Author

FreekVR commented Dec 7, 2023

Hi!

Thanks for the heads-up(s)

Oh, if you're using the Site URL Override setting, that only allows you to provide a single site URL to override, so I'm not surprised that it wouldn't work properly with multiple sites.

The only solution I can think of for this specific type of setup would be either:

1. Make sure your Site URLs are set to what they should be for each frontend site (and that you have your cpSiteUrl set for the CP), and **don't** use the Site URL Override setting

2. We make the config setting for Site URL Override be able to pass in an array of site URLs to override, indexed by the language handle or such

#1 would be a much preferred solution here though @jornwildenbeest

This was configured for our site too, but apparently not needed, as indeed the Craft vanilla site URLs are already correct for us. Removing this fixed the issue!

Hi @khalwat,

Thanks for the quick response!

Looks promising, but with Craft 4.4.17 and php 8.1 I am getting the following error:

2023-12-07 06:25:46 [web.ERROR] [TypeError] TypeError: craft\helpers\UrlHelper::urlWithParams(): Argument #2 ($params) must be of type array|string, null given, called in /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/UrlHelper.php on line 58 and defined in /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/helpers/UrlHelper.php:101
Stack trace:
#0 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/UrlHelper.php(58): craft\helpers\UrlHelper::urlWithParams('https://my-bac...', NULL)
#1 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/DynamicMeta.php(359): nystudio107\seomatic\helpers\UrlHelper::siteUrl('', NULL, NULL, 2)
#2 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/DynamicMeta.php(285): nystudio107\seomatic\helpers\DynamicMeta::addMetaJsonLdBreadCrumbs(2)
#3 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/services/MetaContainers.php(422): nystudio107\seomatic\helpers\DynamicMeta::addDynamicMetaToContainers('', 2)
#4 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/services/MetaContainers.php(305): nystudio107\seomatic\services\MetaContainers->loadMetaContainers('', 2)
#5 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/Container.php(106): nystudio107\seomatic\services\MetaContainers->previewMetaContainers('', 2, true)
#6 [internal function]: nystudio107\seomatic\helpers\Container::nystudio107\seomatic\helpers\{closure}(Object(craft\cache\FileCache))
#7 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/caching/Cache.php(607): call_user_func(Object(Closure), Object(craft\cache\FileCache))
#8 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/Container.php(97): yii\caching\Cache->getOrSet('seomatic_metaco...', Object(Closure), 1, Object(yii\caching\TagDependency))
#9 /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/gql/resolvers/SeomaticResolver.php(64): nystudio107\seomatic\helpers\Container::getContainerArrays(Array, '', 2, true)
#10 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(623): nystudio107\seomatic\gql\resolvers\SeomaticResolver::resolve(NULL, Array, Array, Object(GraphQL\Type\Definition\ResolveInfo))
#11 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(550): GraphQL\Executor\ReferenceExecutor->resolveFieldValueOrError(Object(GraphQL\Type\Definition\FieldDefinition), Object(GraphQL\Language\AST\FieldNode), 'nystudio107\\seo...', NULL, Object(GraphQL\Type\Definition\ResolveInfo))
#12 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(1195): GraphQL\Executor\ReferenceExecutor->resolveField(Object(GraphQL\Type\Definition\ObjectType), NULL, Object(ArrayObject), Array)
#13 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(264): GraphQL\Executor\ReferenceExecutor->executeFields(Object(GraphQL\Type\Definition\ObjectType), NULL, Array, Object(ArrayObject))
#14 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php(215): GraphQL\Executor\ReferenceExecutor->executeOperation(Object(GraphQL\Language\AST\OperationDefinitionNode), NULL)
#15 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/Executor/Executor.php(156): GraphQL\Executor\ReferenceExecutor->doExecute()
#16 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/GraphQL.php(162): GraphQL\Executor\Executor::promiseToExecute(Object(GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter), Object(GraphQL\Type\Schema), Object(GraphQL\Language\AST\DocumentNode), NULL, Array, Array, NULL, NULL)
#17 /Users/jorn/Sites/my-backend-site/vendor/webonyx/graphql-php/src/GraphQL.php(94): GraphQL\GraphQL::promiseToExecute(Object(GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter), Object(GraphQL\Type\Schema), '{\n  seomatic(ur...', NULL, Array, Array, NULL, NULL, Array)
#18 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/services/Gql.php(512): GraphQL\GraphQL::executeQuery(Object(GraphQL\Type\Schema), '{\n  seomatic(ur...', NULL, Array, Array, NULL, NULL, Array)
#19 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/controllers/GraphqlController.php(181): craft\services\Gql->executeQuery(Object(craft\models\GqlSchema), '{\n  seomatic(ur...', Array, NULL, true)
#20 [internal function]: craft\controllers\GraphqlController->actionApi()
#21 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#22 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams(Array)
#23 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/Module.php(552): yii\base\Controller->runAction('api', Array)
#24 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/web/Application.php(304): yii\base\Module->runAction('graphql/api', Array)
#25 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/web/Application.php(607): craft\web\Application->runAction('graphql/api', Array)
#26 /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/web/Application.php(283): craft\web\Application->_processActionRequest(Object(craft\web\Request))
#27 /Users/jorn/Sites/my-backend-site/vendor/yiisoft/yii2/base/Application.php(384): craft\web\Application->handleRequest(Object(craft\web\Request))
#28 /Users/jorn/Sites/my-backend-site/web/index.php(12): yii\base\Application->run()
#29 /Users/jorn/.composer/vendor/laravel/valet/server.php(250): require('/Users/jorn/Sit...')
#30 {main} {"memory":59509384,"exception":"[object] (TypeError(code: 0): craft\\helpers\\UrlHelper::urlWithParams(): Argument #2 ($params) must be of type array|string, null given, called in /Users/jorn/Sites/my-backend-site/vendor/nystudio107/craft-seomatic/src/helpers/UrlHelper.php on line 58 at /Users/jorn/Sites/my-backend-site/vendor/craftcms/cms/src/helpers/UrlHelper.php:101)"} 
2023-12-07 06:25:46 [web.INFO] [application] Request context:
$_GET = [
    'site' => 'default'

Just tried this one too but running into the same error unfortunately.

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

@FreekVR Simple fix, sorry about that

Addressed in: 5fbb544 & 1cdd96b

Just update again, and you'll get the fix:

Craft CMS 3:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop as 3.4.68”,

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop-v4 as 4.0.37”,

Then do a composer clear-cache && composer update

@jornwildenbeest
Copy link

@khalwat

Excellent, thanks for the quick fix!

I can confirm this is working!

@jornwildenbeest
Copy link

One more thing tho @khalwat

I noticed that after updating the canonical tag for another site is looking like this:

<link data-n-head="ssr" href="/fr/fr/fr/des-produits/produits/" rel="canonical">

Instead of what is was before

<link data-n-head="ssr" href="https://my-frontend-url.com/fr/des-produits/produits/" rel="canonical">

Downgrading back solves it.

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

@jornwildenbeest Go to SEOmatic -> Content SEO for the section in question, and tell me what is in the Canonical URL setting there?

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

Also, did you put fully qualified absolute (not relative) URLs in your config/seomatic.php file for the siteUrlOverride settings array?

@jornwildenbeest
Copy link

Hmm, the SEOmatic -> Content SEO -> Canonical URL for the section is set to {entry.url}

For some reason it indeed used relative urls in the config/seomatic.php, so I fixed that one.

Canonical URL for the primary site is working fine, but for other sites it now looks likes this:

https://my-frontend-url.com/fr/fr/des-produits/produits/>

Still a double '/fr'

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

So maybe for your particular setup, you can remove the language prefix from the SEOmatic config/seomatic.php setting? https://my-frontend-url.com/fr/ -> https://my-frontend-url.com/ since for those entries, it appears to be getting the language prefix from {entry.url}?

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

...or you can remove the /fr/ prefix from your Site settings?

@jornwildenbeest
Copy link

Sorry, with the SEOmatic config/seomatic.php setting I mean the siteUrlOverride setting. So no i can't remove the language prefix there because then the hreflang tag would not work correctly.

The problem occurs after setting the siteUrlOverride to the multi site array. Reverting it back to a string with the base url solves canonical problem, but then the hreflang is not working properly.

Could it be that there is still a small bug in the new function causing this bug for the canonical url?

@jornwildenbeest
Copy link

in my case, removing the /fr/ prefix from site settings would cause problems with the admin panel application that we are running besides the headless application, so I'm afraid that is not an option.

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

Well, something isn't quite right here... I'm unable to reproduce the issue locally, so I'm guessing something is different in our config.

For your Section settings, what do you have the URI format set to? Here's an example of mine:

Screenshot 2023-12-07 at 15 37 27

Note the lack of any kind of language prefix, since that is presumed to be taken care of by the Site URL.

@jornwildenbeest
Copy link

jornwildenbeest commented Dec 7, 2023

That's strange..

Section settings:

image

With my SEOmatic settings from the project.yaml file in case it is helpful:

  seomatic:
    enabled: '1'
    licenseKey:
    schemaVersion: 3.0.12
    settings:
      addHrefLang: true
      addPaginatedHreflang: true
      addXDefaultHrefLang: true
      alwaysIncludeCanonicalUrls: false
      cpTitlePrefix: '⚙ '
      cspNonce: ''
      cspScriptSrcPolicies:
        -
          __assoc__:
            -
              - policy
              - '''self'''
      devModeCpTitlePrefix: '&#x1f6a7;⚙ '
      devModeTitlePrefix: '[devMode] '
      displayAnalysisSidebar: true
      displayPreviewSidebar: true
      enableJsonLdEndpoint: false
      enableMetaContainerEndpoint: false
      enableSeoFileLinkEndpoint: false
      environment: local
      excludeNonCanonicalUrls: false
      generatorEnabled: true
      headersEnabled: true
      includeHomepageInBreadcrumbs: true
      lowercaseCanonicalUrl: true
      manuallySetEnvironment: false
      maxDescriptionLength: 155
      maxTitleLength: 55
      metaCacheDuration: 0
      pluginName: SEO
      regenerateSitemapsAutomatically: true
      renderEnabled: true
      separatorChar: '|'
      sidebarDisplayPreviewTypes:
        - google
        - twitter
        - facebook
      siteGroupsSeparate: true
      siteUrlOverride:
        __assoc__:
          -
            - default
            - 'https://my-frontend-url.com/'
          -
            - frontendDe
            - 'https://my-frontend-url.com/de/'
          -
            - frontendFr
            - 'https://my-frontend-url.com/fr/'
      sitemapsEnabled: true
      socialMediaPreviewTarget: true
      submitSitemaps: true
      truncateDescriptionTags: true
      truncateTitleTags: true

Edit: I've removed some of the siteUrlOverride sites here in the project.yaml settings copy, don't confuse that with the screenshot.

the config/seomatic.php file only holds the siteUrlOverride setting.

@jornwildenbeest
Copy link

Did some debugging and found out that in my case, at one point it calls the src/helpers/UrlHelper.php -> siteUrl function with the following siteUrl and path parameters:

Logged the $siteUrl and $path param right before line:$url = rtrim($siteUrl, '/') . '/' . ltrim($path, '/'); (before line 48).
And it logs the following:

$siteUrl contains https://my-frontend-url.com/fr/
$path contains /fr/des-produits/produits/

So with line $url = rtrim($siteUrl, '/') . '/' . ltrim($path, '/');` it combines these 2..

Not sure in which part of the plugin it is calling the siteUrl function, but maybe it is helpful.

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

The $siteUrl above looks correct. So I guess the question now is why is the $path coming in looking like that. Do you perhaps have a backtrace to show what was calling the UrlHelper::siteUrl() with those parameters?

@khalwat
Copy link
Collaborator

khalwat commented Dec 7, 2023

Okay so I had an epiphany here, I think I know what is going on.

I think much like you set the siteUrlOverrides array in config/seomatic.php to relative URLs, I think also your site URLs are set to relative URLs like /fr/ rather than fully qualified URLs.

So let's say an entry's URI is /blog/some-blog, and your site URL is set to /fr/, happens is Craft resolves your {{ entry.url }} to /fr/blog-some-blog

Then SEOmatic notices that this is not a fully qualified, absolute URLs (which is required for the hreflang links), and it attempts to make it into a fully qualified URL by combining it with the siteUrlOverride, which is set to https://example.com/fr/ so you end up with: https://example.com/fr/fr/blog/some-blog

What you need to do is set your site URLs to be fully qualified absolute URLs, not relative URLs.

@jornwildenbeest
Copy link

Okay, think I figured it out.

On line https://github.com/nystudio107/craft-seomatic/blob/develop-v4/src/services/MetaContainers.php#L328 it calls the siteUrl function with a $canonical variable.

Few lines before that it gets the parsed canonical url: https://github.com/nystudio107/craft-seomatic/blob/develop-v4/src/services/MetaContainers.php#L320

if the Content SEO -> {section} -> Canonical Url setting is set to {entry.url}. $canonicalUrl will contain a full absolute url, which when using multiple sites could lead to a url like https://my-frontend-url.com/fr/des-produits/produits/. Which seems correct if you think about it.

Later in the UrlHelper class it separates the url in pieces: https://github.com/nystudio107/craft-seomatic/blob/develop-v4/src/helpers/UrlHelper.php#L48

And combines the uri (which also contains the /fr/) with a siteUrl variable.

BUT with the new function for siteUrlOverrides, the siteIUrl also contains an absolute url with /fr/, hence the double /fr/

So the solution in this case is: changing the Canonical Url setting from {entry.url} to {entry.uri}

khalwat added a commit that referenced this issue Dec 9, 2023
khalwat added a commit that referenced this issue Dec 9, 2023
@khalwat
Copy link
Collaborator

khalwat commented Dec 9, 2023

I'm mostly confused about what I did wrong in my test setup that I didn't catch this earlier.

@jornwildenbeest I pushed changes that should account for overlapping path and URL segments. Can you switch it back to {entry.url} and give it a go?

Craft CMS 3:

You can try it now by setting your server in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop as 3.4.68”,

Then do a composer clear-cache && composer update

…..

Craft CMS 4:

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-seomatic": "dev-develop-v4 as 4.0.37”,

Then do a composer clear-cache && composer update

@jornwildenbeest
Copy link

@khalwat,

Happens to the best of us ;)

Ah great, seems like everything is working as it should now.

Thanks for the fixes!

@khalwat
Copy link
Collaborator

khalwat commented Dec 12, 2023

btw @jornwildenbeest directly inspired by this particular issue is the Craft Coding Challenge, in case you want to try your hand at writing your own method that combines URLs with overlapping segments:

https://www.craftcodingchallenge.com/challenge-14-testing-santas-patience/

No fair cheating & looking at the SEOmatic code tho 😉

@jornwildenbeest
Copy link

Haha cool! Thanks for letting me know, I might give it a shot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants