backdrop-contrib / metatag

Add structured metadata, aka "meta tags", for various pages on your site.
GNU General Public License v2.0
3 stars 15 forks source link

Canonical URL not added with default configuration #109

Closed dgbruns closed 9 months ago

dgbruns commented 1 year ago

With a new install of the Metatag 1.22.5 and default settings...

Under "Advanced Tags" for "Global" and "Content", the following token is set for "Canonical URL":

[current-page:url:absolute]

Since a token is is provided for the canonical url field by default, I would expect to see

<link rel="canonical" href="absolute_url">

In the head of all pages. However, no rel canonical tag is inserted.

Note that if "Always output canonical, shortlink, and generator meta tags" is enabled in Metatag master settings, a canonical tag is inserted.

Reproducible with Backdrop 1.24.1 and Metatag 1.22.5.

Perhaps I am misunderstanding expected behavior?

argiepiano commented 9 months ago

I have seen this same problem, BUT, the problem is not exactly that the canonical URL is not added to the head. Actually is NOT supposed to be added to the head, but rather to the response headers of the page.

The Canonical URL metatag uses the #attached method backdrop_add_http_header (notice this is NOT backdrop_add_html_head).

Still, this is a problem. If you inspect the Response Header with the browser dev tools you'll see that the canonical URL is not included:

Screen Shot 2023-10-05 at 7 33 06 PM

The main issue is the way Backdrop's metatag module processes the metatag info. Unlike the D7 version, Backdrop's version bypasses the normal way of adding tags to the head (or in this case, adding info to the header) by hard-coding backdrop_add_html_head() in metatag_preprocess_page().

This is not right. But rather than making tons of changes, there is a way to avoid this problem. I'll will submit this in a separate issue.

argiepiano commented 9 months ago

I have provided an issue and PR that fixes this. After patching, the header "link" with the canonical URL is sent to the browser. See #110 and #111

quicksketch commented 9 months ago

Actually is NOT supposed to be added to the head, but rather to the response headers of the page.

Although this can be done with the HTTP Link header (per Google documentation), I don't think that's the intention of the current code, which intends to output an HTML element.

It looks to me like the problem here is a mismatch between metatag_html_head_alter() (where the canonical tag is removed) and metatag_preprocess_page() (where metatag adds its own tags). The current code is striping out both the canonical tag added by core, and the canonical tag added by metatag, resulting in no canonical tag at all. If the "Always output canonical, shortlink, and generator meta tags" option is disabled, you actually end up with two canonical tags right now.

In metatag_html_head_alter() we have this code:

      // Do not remove meta tags provided by Metatag.
      if (strpos($key, 'metatag_') === 0) {
        continue;
      }

But in metatag_preprocess_page() no keys are prefixed with metatag_. I'll file a PR to fix this specific issue.

quicksketch commented 9 months ago

Fixed in https://github.com/backdrop-contrib/metatag/pull/114. I'm going to follow-up with test coverage in https://github.com/backdrop-contrib/metatag/issues/115