From: Tom Hughes Date: Thu, 6 Jun 2024 18:09:15 +0000 (+0100) Subject: Merge remote-tracking branch 'upstream/pull/4882' X-Git-Tag: live~448 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/67be6616b0e69fcc275c44b9e8351ddab6e3ce4e?hp=0030c76af085d1055c1cec8ef89f034a87bfe17c Merge remote-tracking branch 'upstream/pull/4882' --- diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb index 9af36709e..f3fbcd1fd 100644 --- a/app/controllers/diary_entries_controller.rb +++ b/app/controllers/diary_entries_controller.rb @@ -68,6 +68,7 @@ class DiaryEntriesController < ApplicationController @entry = entries.find_by(:id => params[:id]) if @entry @title = t ".title", :user => params[:display_name], :title => @entry.title + @og_image = @entry.body.image @comments = can?(:unhidecomment, DiaryEntry) ? @entry.comments : @entry.visible_comments else @title = t "diary_entries.no_such_entry.title", :id => params[:id] diff --git a/app/helpers/open_graph_helper.rb b/app/helpers/open_graph_helper.rb index 27d8d2451..0ce4d6267 100644 --- a/app/helpers/open_graph_helper.rb +++ b/app/helpers/open_graph_helper.rb @@ -1,10 +1,10 @@ module OpenGraphHelper - def opengraph_tags(title = nil) + def opengraph_tags(title = nil, og_image = nil) tags = { "og:site_name" => t("layouts.project_name.title"), "og:title" => [title, t("layouts.project_name.title")].compact.join(" | "), "og:type" => "website", - "og:image" => image_url("osm_logo_256.png"), + "og:image" => og_image ? URI.join(root_url, og_image) : image_url("osm_logo_256.png"), "og:url" => url_for(:only_path => false), "og:description" => t("layouts.intro_text") } diff --git a/app/models/diary_entry.rb b/app/models/diary_entry.rb index e83dfb9ee..089c7e6c6 100644 --- a/app/models/diary_entry.rb +++ b/app/models/diary_entry.rb @@ -50,7 +50,7 @@ class DiaryEntry < ApplicationRecord after_save :spam_check def body - RichText.new(self[:body_format], self[:body]) + @body ||= RichText.new(self[:body_format], self[:body]) end private diff --git a/app/views/layouts/_meta.html.erb b/app/views/layouts/_meta.html.erb index 2352ad0b6..37afbb894 100644 --- a/app/views/layouts/_meta.html.erb +++ b/app/views/layouts/_meta.html.erb @@ -21,7 +21,7 @@ <% end -%> <%= tag.link :rel => "search", :type => "application/opensearchdescription+xml", :title => "OpenStreetMap Search", :href => asset_path("osm.xml") %> <%= tag.meta :name => "description", :content => "OpenStreetMap is the free wiki world map." %> -<%= opengraph_tags(@title) %> +<%= opengraph_tags(@title, @og_image) %> <% if flash[:matomo_goal] -%> <%= tag.meta :name => "matomo-goal", :content => flash[:matomo_goal] %> <% end -%> diff --git a/lib/rich_text.rb b/lib/rich_text.rb index 56d358bd8..f19d3d3a9 100644 --- a/lib/rich_text.rb +++ b/lib/rich_text.rb @@ -49,6 +49,10 @@ module RichText (spammy_phrases * 40) end + def image + nil + end + protected def simple_format(text) @@ -80,12 +84,33 @@ module RichText class Markdown < Base def to_html - linkify(sanitize(Kramdown::Document.new(self).to_html), :all) + linkify(sanitize(document.to_html), :all) end def to_text to_s end + + def image + return @image if defined? @image + + @image = first_image_element(document.root)&.attr&.[]("src") + end + + private + + def document + @document ||= Kramdown::Document.new(self) + end + + def first_image_element(element) + return element if element.type == :img + + element.children.find do |child| + nested_image = first_image_element(child) + break nested_image if nested_image + end + end end class Text < Base diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb index d13a50163..b6d11c62a 100644 --- a/test/controllers/diary_entries_controller_test.rb +++ b/test/controllers/diary_entries_controller_test.rb @@ -752,6 +752,28 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest end end + def test_show_og_image + user = create(:user) + diary_entry = create(:diary_entry, :user => user, :body => "![some picture](https://example.com/picture.jpg)") + + get diary_entry_path(user, diary_entry) + assert_response :success + assert_dom "head meta[property='og:image']" do + assert_dom "> @content", "https://example.com/picture.jpg" + end + end + + def test_show_og_image_with_relative_uri + user = create(:user) + diary_entry = create(:diary_entry, :user => user, :body => "![some local picture](/picture.jpg)") + + get diary_entry_path(user, diary_entry) + assert_response :success + assert_dom "head meta[property='og:image']" do + assert_dom "> @content", "#{root_url}picture.jpg" + end + end + def test_hide user = create(:user) diary_entry = create(:diary_entry, :user => user) diff --git a/test/lib/rich_text_test.rb b/test/lib/rich_text_test.rb index 033a221d4..8dc9e49b1 100644 --- a/test/lib/rich_text_test.rb +++ b/test/lib/rich_text_test.rb @@ -250,6 +250,31 @@ class RichTextTest < ActiveSupport::TestCase assert_equal 141, r.spam_score.round end + def test_text_no_image + r = RichText.new("text", "foo https://example.com/ bar") + assert_nil r.image + end + + def test_html_no_image + r = RichText.new("html", "foo bar baz") + assert_nil r.image + end + + def test_markdown_no_image + r = RichText.new("markdown", "foo [bar](https://example.com/) baz") + assert_nil r.image + end + + def test_markdown_image + r = RichText.new("markdown", "foo ![bar](https://example.com/image.jpg) baz") + assert_equal "https://example.com/image.jpg", r.image + end + + def test_markdown_first_image + r = RichText.new("markdown", "foo ![bar1](https://example.com/image1.jpg) baz\nfoo ![bar2](https://example.com/image2.jpg) baz") + assert_equal "https://example.com/image1.jpg", r.image + end + private def assert_html(richtext, &block)