FIX: use the first image in the first post in the topic as opengraph image

FEATURE: new 'default_opengraph_image_url' setting
This commit is contained in:
Régis Hanol
2015-10-15 11:00:47 +02:00
parent 73e345fe8f
commit 37c5909a31
9 changed files with 29 additions and 25 deletions

View File

@ -122,9 +122,7 @@ module ApplicationHelper
# Creates open graph and twitter card meta data # Creates open graph and twitter card meta data
def crawlable_meta_data(opts=nil) def crawlable_meta_data(opts=nil)
opts ||= {} opts ||= {}
opts[:image] ||= "#{Discourse.base_url}#{SiteSetting.logo_small_url}"
opts[:url] ||= "#{Discourse.base_url_no_prefix}#{request.fullpath}" opts[:url] ||= "#{Discourse.base_url_no_prefix}#{request.fullpath}"
# Use the correct scheme for open graph # Use the correct scheme for open graph
@ -134,21 +132,19 @@ module ApplicationHelper
end end
# Add opengraph tags # Add opengraph tags
result = tag(:meta, property: 'og:site_name', content: SiteSetting.title) << "\n" result = []
result << tag(:meta, property: 'og:site_name', content: SiteSetting.title)
result << tag(:meta, name: 'twitter:card', content: "summary") result << tag(:meta, name: 'twitter:card', content: "summary")
# I removed image related opengraph tags from here for now due to [:url, :title, :description, :image].each do |property|
# https://meta.discourse.org/t/x/22744/18
[:url, :title, :description].each do |property|
if opts[property].present? if opts[property].present?
escape = (property != :image) escape = (property != :image)
result << tag(:meta, {property: "og:#{property}", content: opts[property]}, nil, escape) << "\n" result << tag(:meta, { property: "og:#{property}", content: opts[property] }, nil, escape)
result << tag(:meta, {name: "twitter:#{property}", content: opts[property]}, nil, escape) << "\n" result << tag(:meta, { name: "twitter:#{property}", content: opts[property] }, nil, escape)
end end
end end
result result.join("\n")
end end
# Look up site content for a key. If the key is blank, you can supply a block and that # Look up site content for a key. If the key is blank, you can supply a block and that

View File

@ -49,8 +49,7 @@
<% if @category %> <% if @category %>
<% content_for :head do %> <% content_for :head do %>
<%= auto_discovery_link_tag(:rss, { action: :category_feed }, title: t('rss_topics_in_category', category: @category.name)) %> <%= auto_discovery_link_tag(:rss, { action: :category_feed }, title: t('rss_topics_in_category', category: @category.name)) %>
<%= crawlable_meta_data(title: @category.name, <%= raw crawlable_meta_data(title: @category.name, description: @category.description) %>
description: @category.description) %>
<% end %> <% end %>
<% end %> <% end %>

View File

@ -3,9 +3,7 @@
<head> <head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title><%= @topic_view.topic.title %></title> <title><%= @topic_view.topic.title %></title>
<%= crawlable_meta_data(title: @topic_view.title, <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary, image: @topic_view.image_url) %>
description: @topic_view.summary,
image: @topic_view.image_url) %>
</head> </head>
<body> <body>
<% @topic_view.posts.each do |post| %> <% @topic_view.posts.each do |post| %>

View File

@ -56,9 +56,7 @@
<% content_for :head do %> <% content_for :head do %>
<%= auto_discovery_link_tag(@topic_view, {action: :feed, slug: @topic_view.topic.slug, topic_id: @topic_view.topic.id}, title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %> <%= auto_discovery_link_tag(@topic_view, {action: :feed, slug: @topic_view.topic.slug, topic_id: @topic_view.topic.id}, title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %>
<%= crawlable_meta_data(title: @topic_view.title, <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary, image: @topic_view.image_url) %>
description: @topic_view.summary,
image: @topic_view.image_url) %>
<% end %> <% end %>
<% content_for(:title) { "#{@topic_view.page_title}" } %> <% content_for(:title) { "#{@topic_view.page_title}" } %>

View File

@ -6,9 +6,9 @@
<% content_for :head do %> <% content_for :head do %>
<% if @restrict_fields %> <% if @restrict_fields %>
<%= crawlable_meta_data(title: @user.username, image: @user.small_avatar_url) %> <%= raw crawlable_meta_data(title: @user.username, image: @user.small_avatar_url) %>
<% else %> <% else %>
<%= crawlable_meta_data(title: @user.username, description: @user.user_profile.bio_summary, image: @user.small_avatar_url) %> <%= raw crawlable_meta_data(title: @user.username, description: @user.user_profile.bio_summary, image: @user.small_avatar_url) %>
<% end %> <% end %>
<% end %> <% end %>

View File

@ -1002,6 +1002,8 @@ en:
external_system_avatars_enabled: "Use external system avatars service." external_system_avatars_enabled: "Use external system avatars service."
external_system_avatars_url: "URL of the external system avatars service. Allowed substitions are {username} {first_letter} {color} {size}" external_system_avatars_url: "URL of the external system avatars service. Allowed substitions are {username} {first_letter} {color} {size}"
default_opengraph_image_url: "URL of the default opengraph image."
enable_flash_video_onebox: "Enable embedding of swf and flv (Adobe Flash) links in oneboxes. WARNING: may introduce security risks." enable_flash_video_onebox: "Enable embedding of swf and flv (Adobe Flash) links in oneboxes. WARNING: may introduce security risks."
default_invitee_trust_level: "Default trust level (0-4) for invited users." default_invitee_trust_level: "Default trust level (0-4) for invited users."

View File

@ -593,6 +593,7 @@ files:
default: "https://avatars.discourse.org/v2/letter/{first_letter}/{color}/{size}.png" default: "https://avatars.discourse.org/v2/letter/{first_letter}/{color}/{size}.png"
client: true client: true
regex: '^https?:\/\/.+[^\/]' regex: '^https?:\/\/.+[^\/]'
default_opengraph_image_url: ''
trust: trust:
default_trust_level: default_trust_level:

View File

@ -64,7 +64,7 @@ class CookedPostProcessor
convert_to_link!(img) convert_to_link!(img)
end end
update_topic_image(images) update_topic_image
end end
def extract_images def extract_images
@ -80,6 +80,17 @@ class CookedPostProcessor
@doc.css(".quote img") @doc.css(".quote img")
end end
def extract_images_for_topic
# all image with a src attribute
@doc.css("img[src]") -
# minus, emojis
@doc.css("img.emoji") -
# minus, image inside oneboxes
oneboxed_images -
# minus, images inside quotes
@doc.css(".quote img")
end
def oneboxed_images def oneboxed_images
@doc.css(".onebox-result img, .onebox img") @doc.css(".onebox-result img, .onebox img")
end end
@ -235,9 +246,9 @@ class CookedPostProcessor
span span
end end
def update_topic_image(images) def update_topic_image
if @post.is_first_post? if @post.is_first_post?
img = images.first img = extract_images_for_topic.first
@post.topic.update_column(:image_url, img["src"][0...255]) if img["src"].present? @post.topic.update_column(:image_url, img["src"][0...255]) if img["src"].present?
end end
end end

View File

@ -160,8 +160,7 @@ class TopicView
end end
def image_url def image_url
return nil if desired_post.blank? @topic.image_url || SiteSetting.default_opengraph_image_url
desired_post.user.try(:small_avatar_url)
end end
def filter_posts(opts = {}) def filter_posts(opts = {})