From 580211ee64ea0978f226589c6615a66666b7173f Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 27 Jun 2020 19:07:34 -0500 Subject: [PATCH] seo: fix canonical tags on post index and show page. * Fix incorrect canonical tags. Before we were using ``. This is wrong, it should have been ``. * Add a default canonical tag on all pages. Fixes Google treating the same content on different subdomains (safebooru, shima, kagamihara, etc) as duplicate content. Also fixes Google sometimes treating similar but distinct content on the same domain as duplicate content. --- app/helpers/application_helper.rb | 13 ++++++++ app/views/layouts/default.html.erb | 1 + .../partials/index/_seo_meta_tags.html.erb | 6 ++-- app/views/posts/show.html.erb | 5 ++- test/functional/posts_controller_test.rb | 33 ++++++++++++++++--- test/test_helper.rb | 1 + 6 files changed, 49 insertions(+), 10 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 7c010de1d..4ad0202d8 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -340,6 +340,19 @@ module ApplicationHelper end end + def canonical_url(url = nil) + if url.present? + content_for(:canonical_url) { url } + elsif content_for(:canonical_url).present? + content_for(:canonical_url) + else + request_params = request.params.sort.to_h.with_indifferent_access + request_params.delete(:page) if request_params[:page].to_i == 1 + request_params.delete(:limit) + url_for(**request_params, host: Danbooru.config.hostname, only_path: false) + end + end + def atom_feed_tag(title, url = {}) content_for(:html_header, auto_discovery_link_tag(:atom, url, title: title)) end diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index bf1a94442..ea2c05e17 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -5,6 +5,7 @@ <%= render "meta_links", collection: @current_item %> + <%= tag.link rel: "canonical", href: canonical_url %> <%= csrf_meta_tag %> <% unless CurrentUser.enable_desktop_mode? %> diff --git a/app/views/posts/partials/index/_seo_meta_tags.html.erb b/app/views/posts/partials/index/_seo_meta_tags.html.erb index 8fbfe0a8c..1c58f8d5a 100644 --- a/app/views/posts/partials/index/_seo_meta_tags.html.erb +++ b/app/views/posts/partials/index/_seo_meta_tags.html.erb @@ -10,6 +10,10 @@ <% atom_feed_tag "Posts: #{@post_set.tag_string}", posts_url(tags: @post_set.tag_string, format: :atom) %> <% end %> +<% if params[:tags].blank? && @post_set.current_page == 1 %> + <% canonical_url root_url(host: Danbooru.config.hostname) %> +<% end %> + <% if @post_set.hide_from_crawler? %> <% end %> @@ -18,8 +22,6 @@ <% end %> -<%= tag.meta name: "canonical", content: posts_url(tags: params[:tags], host: Danbooru.config.hostname, protocol: "https") %> - <% if @post_set.best_post.present? %> <%= tag.meta property: "og:image", content: @post_set.best_post.open_graph_image_url %> <%= tag.meta name: "twitter:image", content: @post_set.best_post.open_graph_image_url %> diff --git a/app/views/posts/show.html.erb b/app/views/posts/show.html.erb index 3aa5e7798..9a4112cfe 100644 --- a/app/views/posts/show.html.erb +++ b/app/views/posts/show.html.erb @@ -1,7 +1,8 @@ <% page_title @post.presenter.humanized_essential_tag_string %> <% meta_description "View this #{@post.image_width}x#{@post.image_height} #{number_to_human_size(@post.file_size)} image" %> - +<% canonical_url post_url(@post, host: Danbooru.config.hostname) %> <% atom_feed_tag "Comments for post ##{@post.id}", comments_url(:atom, search: { post_id: @post.id }) %> + <%= render "posts/partials/common/secondary_links" %> <% content_for(:sidebar) do %> @@ -148,8 +149,6 @@ <%= tag.meta property: "og:image", content: @post.open_graph_image_url %> <% end %> - <%= tag.meta name: "canonical", content: post_url(@post, host: Danbooru.config.hostname, protocol: "https") %> - <% if @post.twitter_card_supported? %> diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 41bdc1c88..1e673a532 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -1,6 +1,10 @@ require "test_helper" class PostsControllerTest < ActionDispatch::IntegrationTest + def assert_canonical_url_equals(expected) + assert_equal(expected, response.parsed_body.css("link[rel=canonical]").attribute("href").value) + end + context "The posts controller" do setup do @user = travel_to(1.month.ago) {create(:user)} @@ -10,11 +14,29 @@ class PostsControllerTest < ActionDispatch::IntegrationTest context "index action" do setup do mock_post_search_rankings(Date.today, [["1girl", 100], ["original", 50]]) + create_list(:post, 2) end - should "render" do - get posts_path - assert_response :success + context "for an empty search" do + should "render the first page" do + get root_path + assert_response :success + assert_canonical_url_equals(root_url(host: Danbooru.config.hostname)) + + get posts_path + assert_response :success + assert_canonical_url_equals(root_url(host: Danbooru.config.hostname)) + + get posts_path(page: 1) + assert_response :success + assert_canonical_url_equals(root_url(host: Danbooru.config.hostname)) + end + + should "render the second page" do + get posts_path(page: 2, limit: 1) + assert_response :success + assert_canonical_url_equals(posts_url(page: 2, host: Danbooru.config.hostname)) + end end context "with a single tag search" do @@ -22,6 +44,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest get posts_path, params: { tags: "does_not_exist" } assert_response :success assert_select "#show-excerpt-link", count: 0 + assert_canonical_url_equals(posts_url(tags: "does_not_exist", host: Danbooru.config.hostname)) end should "render for an artist tag" do @@ -261,7 +284,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest get posts_path(format: :atom) assert_response :success - assert_select "entry", 1 + assert_select "entry", 3 end should "render with tags" do @@ -272,7 +295,7 @@ class PostsControllerTest < ActionDispatch::IntegrationTest end should "hide restricted posts" do - @post.update(is_banned: true) + Post.update_all(is_banned: true) get posts_path(format: :atom) assert_response :success diff --git a/test/test_helper.rb b/test/test_helper.rb index 892046f7e..6fed3f46b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -64,6 +64,7 @@ class ActionDispatch::IntegrationTest extend ControllerHelper register_encoder :xml, response_parser: ->(body) { Nokogiri.XML(body) } + register_encoder :html, response_parser: ->(body) { Nokogiri.HTML5(body) } def method_authenticated(method_name, url, user, **options) post session_path, params: { name: user.name, password: user.password }