From 37c5909a3175651f60ea69cf2a863f7c63df8669 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Thu, 15 Oct 2015 11:00:47 +0200
Subject: [PATCH] FIX: use the first image in the first post in the topic as
 opengraph image FEATURE: new 'default_opengraph_image_url' setting

---
 app/helpers/application_helper.rb | 16 ++++++----------
 app/views/list/list.erb           |  3 +--
 app/views/topics/plain.html.erb   |  4 +---
 app/views/topics/show.html.erb    |  4 +---
 app/views/users/show.html.erb     |  4 ++--
 config/locales/server.en.yml      |  2 ++
 config/site_settings.yml          |  1 +
 lib/cooked_post_processor.rb      | 17 ++++++++++++++---
 lib/topic_view.rb                 |  3 +--
 9 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index aa56543de..5b1a863f2 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -122,9 +122,7 @@ module ApplicationHelper
 
   # Creates open graph and twitter card meta data
   def crawlable_meta_data(opts=nil)
-
     opts ||= {}
-    opts[:image] ||= "#{Discourse.base_url}#{SiteSetting.logo_small_url}"
     opts[:url] ||= "#{Discourse.base_url_no_prefix}#{request.fullpath}"
 
     # Use the correct scheme for open graph
@@ -134,21 +132,19 @@ module ApplicationHelper
     end
 
     # 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")
 
-    # I removed image related opengraph tags from here for now due to
-    # https://meta.discourse.org/t/x/22744/18
-    [:url, :title, :description].each do |property|
+    [:url, :title, :description, :image].each do |property|
       if opts[property].present?
         escape = (property != :image)
-        result << tag(:meta, {property: "og:#{property}", content: opts[property]}, nil, escape) << "\n"
-        result << tag(:meta, {name: "twitter:#{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)
       end
     end
 
-    result
+    result.join("\n")
   end
 
   # Look up site content for a key. If the key is blank, you can supply a block and that
diff --git a/app/views/list/list.erb b/app/views/list/list.erb
index b9ae8a086..6bbcebe5b 100644
--- a/app/views/list/list.erb
+++ b/app/views/list/list.erb
@@ -49,8 +49,7 @@
 <% if @category %>
   <% content_for :head do %>
     <%= auto_discovery_link_tag(:rss, { action: :category_feed }, title: t('rss_topics_in_category', category: @category.name)) %>
-    <%= crawlable_meta_data(title: @category.name,
-                            description: @category.description) %>
+    <%= raw crawlable_meta_data(title: @category.name, description: @category.description) %>
   <% end %>
 <% end %>
 
diff --git a/app/views/topics/plain.html.erb b/app/views/topics/plain.html.erb
index 88385ff7d..61895ddd5 100644
--- a/app/views/topics/plain.html.erb
+++ b/app/views/topics/plain.html.erb
@@ -3,9 +3,7 @@
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
   <title><%= @topic_view.topic.title %></title>
-  <%= crawlable_meta_data(title: @topic_view.title,
-                          description: @topic_view.summary,
-                          image: @topic_view.image_url) %>
+  <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary, image: @topic_view.image_url) %>
 </head>
 <body>
 <% @topic_view.posts.each do |post| %>
diff --git a/app/views/topics/show.html.erb b/app/views/topics/show.html.erb
index 9478eefd8..b4d0a95cc 100644
--- a/app/views/topics/show.html.erb
+++ b/app/views/topics/show.html.erb
@@ -56,9 +56,7 @@
 
 <% 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') %>
-  <%= crawlable_meta_data(title: @topic_view.title,
-                          description: @topic_view.summary,
-                          image: @topic_view.image_url) %>
+  <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary, image: @topic_view.image_url) %>
 <% end %>
 
 <% content_for(:title) { "#{@topic_view.page_title}" } %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 8dfa39074..2616716c0 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -6,9 +6,9 @@
 
 <% content_for :head do %>
   <% 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 %>
-    <%= 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 %>
 
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 308aeb395..8996a9b46 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1002,6 +1002,8 @@ en:
     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}"
 
+    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."
 
     default_invitee_trust_level: "Default trust level (0-4) for invited users."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 721796249..73c801b2c 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -593,6 +593,7 @@ files:
     default: "https://avatars.discourse.org/v2/letter/{first_letter}/{color}/{size}.png"
     client: true
     regex: '^https?:\/\/.+[^\/]'
+  default_opengraph_image_url: ''
 
 trust:
   default_trust_level:
diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index 01b20c6f6..ad0a30633 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -64,7 +64,7 @@ class CookedPostProcessor
       convert_to_link!(img)
     end
 
-    update_topic_image(images)
+    update_topic_image
   end
 
   def extract_images
@@ -80,6 +80,17 @@ class CookedPostProcessor
     @doc.css(".quote img")
   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
     @doc.css(".onebox-result img, .onebox img")
   end
@@ -235,9 +246,9 @@ class CookedPostProcessor
     span
   end
 
-  def update_topic_image(images)
+  def update_topic_image
     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?
     end
   end
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index 2a9e70cd9..5ff057dcc 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -160,8 +160,7 @@ class TopicView
   end
 
   def image_url
-    return nil if desired_post.blank?
-    desired_post.user.try(:small_avatar_url)
+    @topic.image_url || SiteSetting.default_opengraph_image_url
   end
 
   def filter_posts(opts = {})