From 230fd5cf8beca3b8df4170f727b92c2677262f48 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 4 Sep 2020 11:25:22 +0100 Subject: [PATCH 1/5] Fix incorrect meta tags The `property` attribute is non-standard for `` elements, and is only intended to be used for opengraph meta tags (taken from RDFa). The correct attribute for other meta tags [1] (including Twitter's meta tags [2]) is `name`. Split the opengraph meta tags into a separate function which we can call from the view, allowing us to loop over the 'standard' and 'opengraph' meta tags separately, using `name` and `property` respectively. [1]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta#Attributes [2]: https://developer.twitter.com/en/docs/twitter-for-websites/cards/guides/getting-started --- example/source/index.html.md.erb | 1 + lib/govuk_tech_docs/meta_tags.rb | 21 +++++++--- lib/source/layouts/core.erb | 6 ++- spec/features/integration_spec.rb | 1 + spec/govuk_tech_docs/meta_tags_spec.rb | 56 ++++++++++++++++++-------- 5 files changed, 61 insertions(+), 24 deletions(-) diff --git a/example/source/index.html.md.erb b/example/source/index.html.md.erb index 528d67e9..00105684 100644 --- a/example/source/index.html.md.erb +++ b/example/source/index.html.md.erb @@ -1,5 +1,6 @@ --- title: GOV.UK Documentation Example +description: Example of a documentation homepage old_paths: - /something/old-as-well.html --- diff --git a/lib/govuk_tech_docs/meta_tags.rb b/lib/govuk_tech_docs/meta_tags.rb index 25d69bd1..ade96476 100644 --- a/lib/govuk_tech_docs/meta_tags.rb +++ b/lib/govuk_tech_docs/meta_tags.rb @@ -8,12 +8,6 @@ def initialize(config, current_page) def tags all_tags = { "description" => page_description, - "og:description" => page_description, - "og:image" => page_image, - "og:site_name" => site_name, - "og:title" => page_title, - "og:type" => "object", - "og:url" => canonical_url, "twitter:card" => "summary", "twitter:domain" => URI.parse(host).host, "twitter:image" => page_image, @@ -24,6 +18,21 @@ def tags Hash[all_tags.select { |_k, v| v }] end + # OpenGraph uses the non-standard property attribute instead of name, so we + # return these separately so we can output them correctly. + def opengraph_tags + all_opengraph_tags = { + "og:description" => page_description, + "og:image" => page_image, + "og:site_name" => site_name, + "og:title" => page_title, + "og:type" => "object", + "og:url" => canonical_url, + } + + Hash[all_opengraph_tags.select { |_k, v| v }] + end + def browser_title [page_title, site_name].select(&:present?).uniq.join(" - ") end diff --git a/lib/source/layouts/core.erb b/lib/source/layouts/core.erb index 2237909b..d7b05c43 100644 --- a/lib/source/layouts/core.erb +++ b/lib/source/layouts/core.erb @@ -18,7 +18,11 @@ <% end %> - <% meta_tags.tags.each do |property, content| %> + <% meta_tags.tags.each do |name, content| %> + <%= tag :meta, name: name, content: content %> + <% end %> + + <% meta_tags.opengraph_tags.each do |property, content| %> <%= tag :meta, property: property, content: content %> <% end %> diff --git a/spec/features/integration_spec.rb b/spec/features/integration_spec.rb index 878bf2ad..f2089ac3 100644 --- a/spec/features/integration_spec.rb +++ b/spec/features/integration_spec.rb @@ -58,6 +58,7 @@ def then_there_is_a_sidebar def and_there_are_proper_meta_tags expect(page).to have_title "GOV.UK Documentation Example - My First Service" + expect(page).to have_css 'meta[name="description"]', visible: false expect(page).to have_css 'meta[property="og:site_name"]', visible: false end diff --git a/spec/govuk_tech_docs/meta_tags_spec.rb b/spec/govuk_tech_docs/meta_tags_spec.rb index b8463d86..99dcd9f7 100644 --- a/spec/govuk_tech_docs/meta_tags_spec.rb +++ b/spec/govuk_tech_docs/meta_tags_spec.rb @@ -32,7 +32,7 @@ def generate_title(site_name:, page_title:) end describe "#tags" do - it "returns all the extra meta tags" do + it "returns standard meta tags" do config = generate_config( host: "https://www.example.org", service_name: "Foo", @@ -46,18 +46,14 @@ def generate_title(site_name:, page_title:) tags = GovukTechDocs::MetaTags.new(config, current_page).tags - expect(tags).to eql("description" => "The description.", - "og:description" => "The description.", - "og:image" => "https://www.example.org/images/govuk-large.png", - "og:site_name" => "Test Site", - "og:title" => "The Title", - "og:type" => "object", - "og:url" => "https://www.example.org/foo.html", + expect(tags).to eql( + "description" => "The description.", "twitter:card" => "summary", "twitter:domain" => "www.example.org", "twitter:image" => "https://www.example.org/images/govuk-large.png", "twitter:title" => "The Title - Test Site", - "twitter:url" => "https://www.example.org/foo.html") + "twitter:url" => "https://www.example.org/foo.html", + ) end it "uses the local variable as page description for proxied pages" do @@ -71,28 +67,54 @@ def generate_title(site_name:, page_title:) expect(tags["description"]).to eql("The local variable description.") end - it "uses the local variable as page title for proxied pages" do + it "works even when no config is set" do current_page = double("current_page", data: double("page_frontmatter", description: "The description.", title: "The Title"), url: "/foo.html", metadata: { locals: { title: "The local variable title." } }) - tags = GovukTechDocs::MetaTags.new(generate_config, current_page).tags + config = { tech_docs: {} } - expect(tags["og:title"]).to eql("The local variable title.") + tags = GovukTechDocs::MetaTags.new(config, current_page).tags + + expect(tags).to be_a(Hash) end + end + + describe "#opengraph_tags" do + it "returns opengraph meta tags" do + config = generate_config( + host: "https://www.example.org", + service_name: "Foo", + full_service_name: "Test Site", + ) - it "works even when no config is set" do current_page = double("current_page", data: double("page_frontmatter", description: "The description.", title: "The Title"), url: "/foo.html", - metadata: { locals: { title: "The local variable title." } }) + metadata: { locals: {} }) - config = { tech_docs: {} } + og_tags = GovukTechDocs::MetaTags.new(config, current_page).opengraph_tags - tags = GovukTechDocs::MetaTags.new(config, current_page).tags + expect(og_tags).to eql( + "og:description" => "The description.", + "og:image" => "https://www.example.org/images/govuk-large.png", + "og:site_name" => "Test Site", + "og:title" => "The Title", + "og:type" => "object", + "og:url" => "https://www.example.org/foo.html", + ) + end - expect(tags).to be_a(Hash) + it "uses the local variable as page title for proxied pages" do + current_page = double("current_page", + data: double("page_frontmatter", description: "The description.", title: "The Title"), + url: "/foo.html", + metadata: { locals: { title: "The local variable title." } }) + + tags = GovukTechDocs::MetaTags.new(generate_config, current_page).opengraph_tags + + expect(tags["og:title"]).to eql("The local variable title.") end end From f49e32cef608d30bf908b503ca600645aaba9f4b Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 4 Sep 2020 11:27:57 +0100 Subject: [PATCH 2/5] Move robots meta tag logic to meta_tags helper --- lib/govuk_tech_docs/meta_tags.rb | 5 +++++ lib/source/layouts/core.erb | 3 --- spec/govuk_tech_docs/meta_tags_spec.rb | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/govuk_tech_docs/meta_tags.rb b/lib/govuk_tech_docs/meta_tags.rb index ade96476..d76d868f 100644 --- a/lib/govuk_tech_docs/meta_tags.rb +++ b/lib/govuk_tech_docs/meta_tags.rb @@ -8,6 +8,7 @@ def initialize(config, current_page) def tags all_tags = { "description" => page_description, + "robots" => robots, "twitter:card" => "summary", "twitter:domain" => URI.parse(host).host, "twitter:image" => page_image, @@ -61,6 +62,10 @@ def page_title locals[:title] || frontmatter.title end + def robots + "noindex" if config[:tech_docs][:prevent_indexing] + end + def host config[:tech_docs][:host].to_s end diff --git a/lib/source/layouts/core.erb b/lib/source/layouts/core.erb index d7b05c43..f19c5148 100644 --- a/lib/source/layouts/core.erb +++ b/lib/source/layouts/core.erb @@ -4,9 +4,6 @@ - <% if config[:tech_docs][:prevent_indexing] %> - - <% end %> <%= meta_tags.browser_title %> diff --git a/spec/govuk_tech_docs/meta_tags_spec.rb b/spec/govuk_tech_docs/meta_tags_spec.rb index 99dcd9f7..820b9b7b 100644 --- a/spec/govuk_tech_docs/meta_tags_spec.rb +++ b/spec/govuk_tech_docs/meta_tags_spec.rb @@ -56,6 +56,21 @@ def generate_title(site_name:, page_title:) ) end + it "adds a noindex robots tag when the site config prevents indexing" do + config = generate_config( + prevent_indexing: true, + ) + + current_page = double("current_page", + data: double("page_frontmatter", description: "The description.", title: "The Title"), + url: "/foo.html", + metadata: { locals: {} }) + + tags = GovukTechDocs::MetaTags.new(config, current_page).tags + + expect(tags["robots"]).to eql("noindex") + end + it "uses the local variable as page description for proxied pages" do current_page = double("current_page", data: double("page_frontmatter", description: "The description.", title: "The Title"), From a10a80e35d72fd130e97bef50b068e13066efde9 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 4 Sep 2020 11:29:22 +0100 Subject: [PATCH 3/5] Move validation meta tag logic to meta tags helper --- lib/govuk_tech_docs/meta_tags.rb | 5 +++++ lib/source/layouts/core.erb | 4 ---- spec/govuk_tech_docs/meta_tags_spec.rb | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/govuk_tech_docs/meta_tags.rb b/lib/govuk_tech_docs/meta_tags.rb index d76d868f..c3ed1738 100644 --- a/lib/govuk_tech_docs/meta_tags.rb +++ b/lib/govuk_tech_docs/meta_tags.rb @@ -8,6 +8,7 @@ def initialize(config, current_page) def tags all_tags = { "description" => page_description, + "google-site-verification" => google_site_verification, "robots" => robots, "twitter:card" => "summary", "twitter:domain" => URI.parse(host).host, @@ -66,6 +67,10 @@ def robots "noindex" if config[:tech_docs][:prevent_indexing] end + def google_site_verification + config[:tech_docs][:google_site_verification] + end + def host config[:tech_docs][:host].to_s end diff --git a/lib/source/layouts/core.erb b/lib/source/layouts/core.erb index f19c5148..64ca034f 100644 --- a/lib/source/layouts/core.erb +++ b/lib/source/layouts/core.erb @@ -11,10 +11,6 @@ - <% if config[:tech_docs][:google_site_verification] %> - - <% end %> - <% meta_tags.tags.each do |name, content| %> <%= tag :meta, name: name, content: content %> <% end %> diff --git a/spec/govuk_tech_docs/meta_tags_spec.rb b/spec/govuk_tech_docs/meta_tags_spec.rb index 820b9b7b..1750d7e8 100644 --- a/spec/govuk_tech_docs/meta_tags_spec.rb +++ b/spec/govuk_tech_docs/meta_tags_spec.rb @@ -71,6 +71,21 @@ def generate_title(site_name:, page_title:) expect(tags["robots"]).to eql("noindex") end + it "adds a google site validation meta tag when provided in config" do + config = generate_config( + google_site_verification: "LEGIT-VALIDATION-TOKEN", + ) + + current_page = double("current_page", + data: double("page_frontmatter", description: "The description.", title: "The Title"), + url: "/foo.html", + metadata: { locals: {} }) + + tags = GovukTechDocs::MetaTags.new(config, current_page).tags + + expect(tags["google-site-verification"]).to eql("LEGIT-VALIDATION-TOKEN") + end + it "uses the local variable as page description for proxied pages" do current_page = double("current_page", data: double("page_frontmatter", description: "The description.", title: "The Title"), From 5b6d2b08e2ceebb21a7e96b0ede4e0da81af5ae9 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 4 Sep 2020 11:34:38 +0100 Subject: [PATCH 4/5] Allow individual pages to have noindex robots tags Make it possible to add noindex robots meta tags to individual pages by adding `prevent_indexing: true` to the page frontmatter. Because the meta_tags helper now checks for the prevent_indexing property on the data object, we would now have to add the `prevent_indexing` attribute to every page_metadata double otherwise we would see the error: > # received unexpected message :prevent_indexing with (no args) To avoid this, replace the current `data` test doubles with hashes, and update the meta_tags helper to use the standard Ruby syntax to read values out of the hashes, rather than relying on the method access [1] feature that we get from current_page.data being a Middleman::Util::EnhancedHash [2]. [1]: https://github.com/hashie/hashie/tree/b24d6dca2c545637bc3cc3ac4d89f565fc27a9d0#methodaccess [2]: https://github.com/middleman/middleman/blob/d7f0ed06d3bf10bd8bb7f9abfda87fcdfbb3c363/middleman-core/lib/middleman-core/util/data.rb#L18 --- example/source/prevent-index-page.html.md | 10 +++++++++ lib/govuk_tech_docs/meta_tags.rb | 6 ++--- spec/features/integration_spec.rb | 11 +++++++++ spec/govuk_tech_docs/meta_tags_spec.rb | 27 ++++++++++++++++------- 4 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 example/source/prevent-index-page.html.md diff --git a/example/source/prevent-index-page.html.md b/example/source/prevent-index-page.html.md new file mode 100644 index 00000000..e3a55482 --- /dev/null +++ b/example/source/prevent-index-page.html.md @@ -0,0 +1,10 @@ +--- +title: Un-indexed Page +prevent_indexing: true +hide_in_navigation: true +--- + +# Un-indexed page + +This page should not be indexed by search engines, because it contains a +`` tag. diff --git a/lib/govuk_tech_docs/meta_tags.rb b/lib/govuk_tech_docs/meta_tags.rb index c3ed1738..76850e1b 100644 --- a/lib/govuk_tech_docs/meta_tags.rb +++ b/lib/govuk_tech_docs/meta_tags.rb @@ -56,15 +56,15 @@ def site_name end def page_description - locals[:description] || frontmatter.description + locals[:description] || frontmatter[:description] end def page_title - locals[:title] || frontmatter.title + locals[:title] || frontmatter[:title] end def robots - "noindex" if config[:tech_docs][:prevent_indexing] + "noindex" if config[:tech_docs][:prevent_indexing] || frontmatter[:prevent_indexing] end def google_site_verification diff --git a/spec/features/integration_spec.rb b/spec/features/integration_spec.rb index f2089ac3..5195172b 100644 --- a/spec/features/integration_spec.rb +++ b/spec/features/integration_spec.rb @@ -34,6 +34,9 @@ when_i_view_a_page_with_no_sidebar then_there_is_no_sidebar + + when_i_view_a_page_with_prevent_indexing + then_there_is_a_robots_noindex_metatag end def when_the_site_is_created @@ -125,4 +128,12 @@ def when_i_view_a_page_with_no_sidebar def then_there_is_no_sidebar expect(page).to have_no_css "div.app-pane__toc" end + + def when_i_view_a_page_with_prevent_indexing + visit "/prevent-index-page.html" + end + + def then_there_is_a_robots_noindex_metatag + expect(page).to have_css 'meta[name="robots"][content="noindex"]', visible: false + end end diff --git a/spec/govuk_tech_docs/meta_tags_spec.rb b/spec/govuk_tech_docs/meta_tags_spec.rb index 1750d7e8..1b1b69fd 100644 --- a/spec/govuk_tech_docs/meta_tags_spec.rb +++ b/spec/govuk_tech_docs/meta_tags_spec.rb @@ -24,7 +24,7 @@ def generate_title(site_name:, page_title:) ) current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: page_title), + data: { description: "The description.", title: page_title }, metadata: { locals: {} }) GovukTechDocs::MetaTags.new(config, current_page).browser_title @@ -40,7 +40,7 @@ def generate_title(site_name:, page_title:) ) current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: { description: "The description.", title: "The Title" }, url: "/foo.html", metadata: { locals: {} }) @@ -62,7 +62,7 @@ def generate_title(site_name:, page_title:) ) current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: {}, url: "/foo.html", metadata: { locals: {} }) @@ -71,13 +71,24 @@ def generate_title(site_name:, page_title:) expect(tags["robots"]).to eql("noindex") end + it "adds a noindex robots tag when the page frontmatter prevents indexing" do + current_page = double("current_page", + data: { prevent_indexing: true }, + url: "/foo.html", + metadata: { locals: {} }) + + tags = GovukTechDocs::MetaTags.new(generate_config, current_page).tags + + expect(tags["robots"]).to eql("noindex") + end + it "adds a google site validation meta tag when provided in config" do config = generate_config( google_site_verification: "LEGIT-VALIDATION-TOKEN", ) current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: {}, url: "/foo.html", metadata: { locals: {} }) @@ -88,7 +99,7 @@ def generate_title(site_name:, page_title:) it "uses the local variable as page description for proxied pages" do current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: { description: "The description." }, url: "/foo.html", metadata: { locals: { description: "The local variable description." } }) @@ -99,7 +110,7 @@ def generate_title(site_name:, page_title:) it "works even when no config is set" do current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: {}, url: "/foo.html", metadata: { locals: { title: "The local variable title." } }) @@ -120,7 +131,7 @@ def generate_title(site_name:, page_title:) ) current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: { description: "The description.", title: "The Title" }, url: "/foo.html", metadata: { locals: {} }) @@ -138,7 +149,7 @@ def generate_title(site_name:, page_title:) it "uses the local variable as page title for proxied pages" do current_page = double("current_page", - data: double("page_frontmatter", description: "The description.", title: "The Title"), + data: { description: "The description." }, url: "/foo.html", metadata: { locals: { title: "The local variable title." } }) From 0ed618bab4f90cc26be011a5d6631abc7aef97d6 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Fri, 14 Aug 2020 17:34:36 +0100 Subject: [PATCH 5/5] Document in CHANGELOG --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 138eedef..fb85796a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## Unreleased + +### New features + +#### Exclude individual pages from search engine indexes + +You can now exclude individual pages from search engine indexes by including `prevent_indexing: true` in the frontmatter for the page. + +This was added in [pull request #192: Fixes and improvements to meta tags](https://github.com/alphagov/tech-docs-gem/pull/192). + +### Fixes + +We’ve made fixes to the Tech Docs Gem in the following pull requests: + +- [#192: Fixes and improvements to meta tags](https://github.com/alphagov/tech-docs-gem/pull/192) + ## 2.0.13 - [Pull request #189: Update orange code highlight colour to meet minimum AA colour contrast ratio criterion](https://github.com/alphagov/tech-docs-gem/pull/189)