Add a canonical URL meta tag #302
Conversation
|
Would this be for every page? I don't know a huge amount about govuk_publishing_components so I may be wrong but off the top of my head pages like https://www.gov.uk/foreign-travel-advice/thailand/safety-and-security have different path to base_path (basically any with a prefix base_path) |
|
@kevindew Yeah, I think it should be for every page. I did have a think about whether to use |
|
Might be worth talking to a content designer about this? Mainstream guides have a bunch of parts with the same base path but different slugs. Some are more coherent as a single entity than others. I'd recommend Jon Sanger |
|
@thomasleese Sure I don't think it's correct that /foreign-travel-advice/thailand/safety-and-security has a canonical of /foreign-travel-advice/thailand as google would not index safety and security in that case. I'm not really sure about request paths either as I'm not sure if there are places where we use query strings to indicate particular content (maybe smart answers but not sure if we index those) My instinct is that this might just have to be not a catch-all solution and more of an opt-in depending on what is being rendered. That's on the basis that I think incorrectly implemented canonical tags will cause more harm than them missing. Do we have a contact in SEO that we can hit up with questions? |
|
@kevindew In terms of query strings, we've decided that content where the user can filter shouldn't be index and that's covered in https://trello.com/c/PHjO8hP2/139-noindex-pagination-from-external-search-2. Smart answers uses paths not query strings to indicate content so that shouldn't be a problem. I'm more in favour of using the request path now as the canonical URL, but it's true that applying this to all pages there are inevitably cases where either option doesn't work and could cause harm. We could have logic in the component that understands how to build the canonical URL of different document types, but that does feel better suited in the individual frontend apps. Based on this and our earlier discussion on Slack @bevanloon and @tijmenb what do you think? |
|
(For context, @kevindew, my initial idea was to have a blacklist of query parameters we know that we want to remove from the request URL to build a canonical URL (i.e. |
|
okey doke, cool re query strings. If we use request path it feels like it's sufficiently dumb it might not provide much value to me. As it stands google uses sitemap to work out what it thinks is the canonical path so I can't imagine it's getting caught mistaking analytics parameters often and then that means if we have instances of rendering same content on multiple pages they'll be flagged as having different canonical paths. Sorry Tom, I know I'm getting more caught up in the value of the task than the implementation which isn't particularly helpful. |
thomasleese
force-pushed
the
add-canonical-url-meta-tag
branch
from
9fc7acd to
9ea8b80
Compare
| @@ -106,6 +107,14 @@ def add_step_by_step_tags(meta_tags) | |||
| meta_tags | |||
| end | |||
|
|
|||
| def add_canonical_tag(meta_tags) | |||
There was a problem hiding this comment.
Can we allow them to specify the base_path through a local assign? and then fall back to base_path?
It might be even clearer still if rather than include_canonical as a boolean we just had canonical_path as a local assign then it'd be less magical and more configurable. What do you think?
There was a problem hiding this comment.
I've pushed up a change. It makes the attribute configurable but if you leave it as true it sticks with the default base path (to avoid having to add too much logic to the frontend apps), what do you think?
There was a problem hiding this comment.
That change looks good to me 👍 - although as it's a path I think we should still prefix with Plek website root when it's configured
There was a problem hiding this comment.
Good point, I've amended that now!
thomasleese
force-pushed
the
add-canonical-url-meta-tag
branch
from
9ea8b80 to
602c29d
Compare
thomasleese
force-pushed
the
add-canonical-url-meta-tag
branch
from
602c29d to
23e197c
Compare
spec/components/meta_tags_spec.rb
Outdated
| it "renders the canonical URL in a meta tag if overridden" do | ||
| render_component( | ||
| content_item: { base_path: "/test" }, | ||
| canonical_path: "http://www.dev.gov.uk/test2", |
thomasleese
force-pushed
the
add-canonical-url-meta-tag
branch
2 times, most recently
from
30e44e9 to
d1d9cfb
Compare
spec/components/meta_tags_spec.rb
Outdated
| assert_empty render_component(content_item: { | ||
| base_path: "/test" | ||
| }).strip | ||
| end |
There was a problem hiding this comment.
I'm sorry I was all set to give the ✅ but there's some quote inconsistencies
There was a problem hiding this comment.
Yeah... I've made them all single quotes I think... I like to think they were consistent with the inconsistent style of the file...
This is an optional meta tag which defaults to not being shown. It's built from the base_path of the content item which means it strips out any query parameters or fragments.
thomasleese
force-pushed
the
add-canonical-url-meta-tag
branch
from
d1d9cfb to
e40ea33
Compare
This is required as the updated meta tag component uses it to build a URL.
|
I'm away today but let's chat tomorrow how this would work. If it's an optional thing it might be better to implement per-app anyway, it's more explicit. I'm not sure apps would know whether or not to add the tag currently. |
…tag" This reverts commit b53fa52, reversing changes made to 8f9ed42. After adding this we realised that it would be better placed for the individual frontend applications to add the canonical tag as they work closer to the content and would understand how to generate different canonical URLs depending on document type and various other factors. Although we could choose to leave it in here, we felt it would be better for apps to be explicit about the tag rather than having a default which might cause more harm than good.
This is built from the base_path of the content item which means it strips out any query parameters or fragments.
Trello Card