From 5de65ce3e89ba2f229170ed18933c99e5caa8dff Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 8 Jun 2021 15:59:55 -0500
Subject: [PATCH 01/15] Set the correct height/width if the data is available
 when generating twittercard metadata

---
 lib/pleroma/web/metadata/providers/twitter_card.ex    |  7 +++++--
 .../web/metadata/providers/twitter_card_test.exs      | 11 ++++++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index 12c372d77..e28f832d4 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -80,11 +80,14 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
             # TODO: Need the true width and height values here or Twitter renders an iFrame with
             # a bad aspect ratio
             "video" ->
+              height = url["height"] || 480
+              width = url["width"] || 480
+
               [
                 {:meta, [property: "twitter:card", content: "player"], []},
                 {:meta, [property: "twitter:player", content: player_url(id)], []},
-                {:meta, [property: "twitter:player:width", content: "480"], []},
-                {:meta, [property: "twitter:player:height", content: "480"], []},
+                {:meta, [property: "twitter:player:width", content: "#{width}"], []},
+                {:meta, [property: "twitter:player:height", content: "#{height}"], []},
                 {:meta, [property: "twitter:player:stream", content: url["href"]], []},
                 {:meta,
                  [property: "twitter:player:stream:content_type", content: url["mediaType"]], []}
diff --git a/test/pleroma/web/metadata/providers/twitter_card_test.exs b/test/pleroma/web/metadata/providers/twitter_card_test.exs
index 196bca20a..6d761f4e4 100644
--- a/test/pleroma/web/metadata/providers/twitter_card_test.exs
+++ b/test/pleroma/web/metadata/providers/twitter_card_test.exs
@@ -123,7 +123,12 @@ test "it renders supported types of attachments and skips unknown types" do
             },
             %{
               "url" => [
-                %{"mediaType" => "video/webm", "href" => "https://pleroma.gov/about/juche.webm"}
+                %{
+                  "mediaType" => "video/webm",
+                  "href" => "https://pleroma.gov/about/juche.webm",
+                  "height" => 600,
+                  "width" => 800
+                }
               ]
             }
           ]
@@ -143,8 +148,8 @@ test "it renders supported types of attachments and skips unknown types" do
                 property: "twitter:player",
                 content: Router.Helpers.o_status_url(Endpoint, :notice_player, activity.id)
               ], []},
-             {:meta, [property: "twitter:player:width", content: "480"], []},
-             {:meta, [property: "twitter:player:height", content: "480"], []},
+             {:meta, [property: "twitter:player:width", content: "800"], []},
+             {:meta, [property: "twitter:player:height", content: "600"], []},
              {:meta,
               [
                 property: "twitter:player:stream",

From d4ac9445cd485a4055f93360714923c3f64d9673 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 8 Jun 2021 16:19:12 -0500
Subject: [PATCH 02/15] Twittercard metadata for images should also include
 dimensions if available

---
 lib/pleroma/web/metadata/providers/twitter_card.ex  | 13 ++++++-------
 .../web/metadata/providers/twitter_card_test.exs    | 11 ++++++++++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index e28f832d4..bf6d4bcbe 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -55,7 +55,9 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
     Enum.reduce(attachments, [], fn attachment, acc ->
       rendered_tags =
         Enum.reduce(attachment["url"], [], fn url, acc ->
-          # TODO: Add additional properties to objects when we have the data available.
+          height = url["height"] || 480
+          width = url["width"] || 480
+
           case Utils.fetch_media_type(@media_types, url["mediaType"]) do
             "audio" ->
               [
@@ -73,16 +75,13 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
                  [
                    property: "twitter:player",
                    content: Utils.attachment_url(url["href"])
-                 ], []}
+                 ], []},
+                {:meta, [property: "twitter:player:width", content: "#{width}"], []},
+                {:meta, [property: "twitter:player:height", content: "#{height}"], []}
                 | acc
               ]
 
-            # TODO: Need the true width and height values here or Twitter renders an iFrame with
-            # a bad aspect ratio
             "video" ->
-              height = url["height"] || 480
-              width = url["width"] || 480
-
               [
                 {:meta, [property: "twitter:card", content: "player"], []},
                 {:meta, [property: "twitter:player", content: player_url(id)], []},
diff --git a/test/pleroma/web/metadata/providers/twitter_card_test.exs b/test/pleroma/web/metadata/providers/twitter_card_test.exs
index 6d761f4e4..dbb15b79f 100644
--- a/test/pleroma/web/metadata/providers/twitter_card_test.exs
+++ b/test/pleroma/web/metadata/providers/twitter_card_test.exs
@@ -111,7 +111,14 @@ test "it renders supported types of attachments and skips unknown types" do
           "content" => "pleroma in a nutshell",
           "attachment" => [
             %{
-              "url" => [%{"mediaType" => "image/png", "href" => "https://pleroma.gov/tenshi.png"}]
+              "url" => [
+                %{
+                  "mediaType" => "image/png",
+                  "href" => "https://pleroma.gov/tenshi.png",
+                  "height" => 1024,
+                  "width" => 1280
+                }
+              ]
             },
             %{
               "url" => [
@@ -142,6 +149,8 @@ test "it renders supported types of attachments and skips unknown types" do
              {:meta, [property: "twitter:description", content: "pleroma in a nutshell"], []},
              {:meta, [property: "twitter:card", content: "summary_large_image"], []},
              {:meta, [property: "twitter:player", content: "https://pleroma.gov/tenshi.png"], []},
+             {:meta, [property: "twitter:player:width", content: "1280"], []},
+             {:meta, [property: "twitter:player:height", content: "1024"], []},
              {:meta, [property: "twitter:card", content: "player"], []},
              {:meta,
               [

From aa8cc4e86e5c7a53fa8bc606dbce6c6b3a0a8c02 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 8 Jun 2021 16:31:12 -0500
Subject: [PATCH 03/15] Only use fallback for videos and only add this metadata
 for images if we really have it.

---
 .../web/metadata/providers/twitter_card.ex    | 28 +++++++++++++++----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index bf6d4bcbe..dfe477a8a 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -55,9 +55,6 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
     Enum.reduce(attachments, [], fn attachment, acc ->
       rendered_tags =
         Enum.reduce(attachment["url"], [], fn url, acc ->
-          height = url["height"] || 480
-          width = url["width"] || 480
-
           case Utils.fetch_media_type(@media_types, url["mediaType"]) do
             "audio" ->
               [
@@ -75,13 +72,16 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
                  [
                    property: "twitter:player",
                    content: Utils.attachment_url(url["href"])
-                 ], []},
-                {:meta, [property: "twitter:player:width", content: "#{width}"], []},
-                {:meta, [property: "twitter:player:height", content: "#{height}"], []}
+                 ], []}
                 | acc
               ]
+              |> maybe_add_dimensions(url)
 
             "video" ->
+              # fallback to old placeholder values
+              height = url["height"] || 480
+              width = url["width"] || 480
+
               [
                 {:meta, [property: "twitter:card", content: "player"], []},
                 {:meta, [property: "twitter:player", content: player_url(id)], []},
@@ -107,4 +107,20 @@ defp build_attachments(_id, _object), do: []
   defp player_url(id) do
     Pleroma.Web.Router.Helpers.o_status_url(Pleroma.Web.Endpoint, :notice_player, id)
   end
+
+  # Videos have problems without dimensions, but we used to not provide WxH for images.
+  # A default (read: incorrect) fallback for images is likely to cause rendering bugs.
+  defp maybe_add_dimensions(metadata, url) do
+    cond do
+      !is_nil(url["height"]) && !is_nil(url["width"]) ->
+        metadata ++
+          [
+            {:meta, [property: "twitter:player:width", content: "#{url["width"]}"], []},
+            {:meta, [property: "twitter:player:height", content: "#{url["height"]}"], []}
+          ]
+
+      true ->
+        metadata
+    end
+  end
 end

From d70db63084449e48e90288bc7484733171246625 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 8 Jun 2021 16:58:33 -0500
Subject: [PATCH 04/15] Set the correct height/width if the data is available
 when generating opengraph metadata

---
 .../web/metadata/providers/open_graph.ex      | 22 +++++++++++++++++--
 .../metadata/providers/open_graph_test.exs    | 20 ++++++++++++++---
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index 18ddde84b..78cef1525 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -69,8 +69,7 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
     Enum.reduce(attachments, [], fn attachment, acc ->
       rendered_tags =
         Enum.reduce(attachment["url"], [], fn url, acc ->
-          # TODO: Add additional properties to objects when we have the data available.
-          # Also, Whatsapp only wants JPEG or PNGs. It seems that if we add a second og:image
+          # TODO: Whatsapp only wants JPEG or PNGs. It seems that if we add a second og:image
           # object when a Video or GIF is attached it will display that in Whatsapp Rich Preview.
           case Utils.fetch_media_type(@media_types, url["mediaType"]) do
             "audio" ->
@@ -85,12 +84,14 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
                 {:meta, [property: "og:image:alt", content: attachment["name"]], []}
                 | acc
               ]
+              |> maybe_add_dimensions(url)
 
             "video" ->
               [
                 {:meta, [property: "og:video", content: Utils.attachment_url(url["href"])], []}
                 | acc
               ]
+              |> maybe_add_dimensions(url)
 
             _ ->
               acc
@@ -102,4 +103,21 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
   end
 
   defp build_attachments(_), do: []
+
+  # We can use url["mediaType"] to dynamically fill the metadata
+  defp maybe_add_dimensions(metadata, url) do
+    type = url["mediaType"] |> String.split("/") |> List.first()
+
+    cond do
+      !is_nil(url["height"]) && !is_nil(url["width"]) ->
+        metadata ++
+          [
+            {:meta, [property: "og:#{type}:width", content: "#{url["width"]}"], []},
+            {:meta, [property: "og:#{type}:height", content: "#{url["height"]}"], []}
+          ]
+
+      true ->
+        metadata
+    end
+  end
 end
diff --git a/test/pleroma/web/metadata/providers/open_graph_test.exs b/test/pleroma/web/metadata/providers/open_graph_test.exs
index fc44b3cbd..f5f71cee5 100644
--- a/test/pleroma/web/metadata/providers/open_graph_test.exs
+++ b/test/pleroma/web/metadata/providers/open_graph_test.exs
@@ -22,7 +22,12 @@ test "it renders all supported types of attachments and skips unknown types" do
           "attachment" => [
             %{
               "url" => [
-                %{"mediaType" => "image/png", "href" => "https://pleroma.gov/tenshi.png"}
+                %{
+                  "mediaType" => "image/png",
+                  "href" => "https://pleroma.gov/tenshi.png",
+                  "height" => 1024,
+                  "width" => 1280
+                }
               ]
             },
             %{
@@ -35,7 +40,12 @@ test "it renders all supported types of attachments and skips unknown types" do
             },
             %{
               "url" => [
-                %{"mediaType" => "video/webm", "href" => "https://pleroma.gov/about/juche.webm"}
+                %{
+                  "mediaType" => "video/webm",
+                  "href" => "https://pleroma.gov/about/juche.webm",
+                  "height" => 600,
+                  "width" => 800
+                }
               ]
             },
             %{
@@ -55,11 +65,15 @@ test "it renders all supported types of attachments and skips unknown types" do
     assert Enum.all?(
              [
                {:meta, [property: "og:image", content: "https://pleroma.gov/tenshi.png"], []},
+               {:meta, [property: "og:image:width", content: "1280"], []},
+               {:meta, [property: "og:image:height", content: "1024"], []},
                {:meta,
                 [property: "og:audio", content: "http://www.gnu.org/music/free-software-song.au"],
                 []},
                {:meta, [property: "og:video", content: "https://pleroma.gov/about/juche.webm"],
-                []}
+                []},
+               {:meta, [property: "og:video:width", content: "800"], []},
+               {:meta, [property: "og:video:height", content: "600"], []}
              ],
              fn element -> element in result end
            )

From 9cb8960284c7046d4a058c0526b55bce5ef9513b Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Tue, 8 Jun 2021 17:14:30 -0500
Subject: [PATCH 05/15] Switch OGP default type from "website" to "article"

This is what Mastodon uses and might fix some link preview bugs I've encountered
---
 lib/pleroma/web/metadata/providers/open_graph.ex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index 78cef1525..e5712ec63 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -32,7 +32,7 @@ def build_tags(%{
          property: "og:description",
          content: scrubbed_content
        ], []},
-      {:meta, [property: "og:type", content: "website"], []}
+      {:meta, [property: "og:type", content: "article"], []}
     ] ++
       if attachments == [] or Metadata.activity_nsfw?(object) do
         [
@@ -57,7 +57,7 @@ def build_tags(%{user: user}) do
          ], []},
         {:meta, [property: "og:url", content: user.uri || user.ap_id], []},
         {:meta, [property: "og:description", content: truncated_bio], []},
-        {:meta, [property: "og:type", content: "website"], []},
+        {:meta, [property: "og:type", content: "article"], []},
         {:meta, [property: "og:image", content: Utils.attachment_url(User.avatar_url(user))], []},
         {:meta, [property: "og:image:width", content: 150], []},
         {:meta, [property: "og:image:height", content: 150], []}

From 19a49dd757ebf60e8501c481f2d2be9d5e326808 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 09:58:29 -0500
Subject: [PATCH 06/15] Remove Metadata.Utils.attachment_url/1

This was a wasteful shortcut to MediaProxy.preview_url/1 and we don't
always want the preview_url in the metadata anyway.
---
 lib/pleroma/web/metadata/providers/open_graph.ex | 16 ++++++++++------
 .../web/metadata/providers/twitter_card.ex       | 12 +++++++++---
 lib/pleroma/web/metadata/utils.ex                |  5 -----
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index e5712ec63..75d155236 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -4,6 +4,7 @@
 
 defmodule Pleroma.Web.Metadata.Providers.OpenGraph do
   alias Pleroma.User
+  alias Pleroma.Web.MediaProxy
   alias Pleroma.Web.Metadata
   alias Pleroma.Web.Metadata.Providers.Provider
   alias Pleroma.Web.Metadata.Utils
@@ -36,8 +37,7 @@ def build_tags(%{
     ] ++
       if attachments == [] or Metadata.activity_nsfw?(object) do
         [
-          {:meta, [property: "og:image", content: Utils.attachment_url(User.avatar_url(user))],
-           []},
+          {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], []},
           {:meta, [property: "og:image:width", content: 150], []},
           {:meta, [property: "og:image:height", content: 150], []}
         ]
@@ -58,7 +58,7 @@ def build_tags(%{user: user}) do
         {:meta, [property: "og:url", content: user.uri || user.ap_id], []},
         {:meta, [property: "og:description", content: truncated_bio], []},
         {:meta, [property: "og:type", content: "article"], []},
-        {:meta, [property: "og:image", content: Utils.attachment_url(User.avatar_url(user))], []},
+        {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], []},
         {:meta, [property: "og:image:width", content: 150], []},
         {:meta, [property: "og:image:height", content: 150], []}
       ]
@@ -74,13 +74,17 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
           case Utils.fetch_media_type(@media_types, url["mediaType"]) do
             "audio" ->
               [
-                {:meta, [property: "og:audio", content: Utils.attachment_url(url["href"])], []}
+                {:meta, [property: "og:audio", content: MediaProxy.url(url["href"])], []}
                 | acc
               ]
 
+            # Not using preview_url for this. It saves bandwidth, but the image dimensions will be wrong.
+            # We generate it on the fly and have no way to capture or analyze the image to get the dimensions.
+            # This can be an issue for apps/FEs rendering images in timelines too, but you can get clever with
+            # the aspect ratio metadata as a workaround.
             "image" ->
               [
-                {:meta, [property: "og:image", content: Utils.attachment_url(url["href"])], []},
+                {:meta, [property: "og:image", content: MediaProxy.url(url["href"])], []},
                 {:meta, [property: "og:image:alt", content: attachment["name"]], []}
                 | acc
               ]
@@ -88,7 +92,7 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
 
             "video" ->
               [
-                {:meta, [property: "og:video", content: Utils.attachment_url(url["href"])], []}
+                {:meta, [property: "og:video", content: MediaProxy.url(url["href"])], []}
                 | acc
               ]
               |> maybe_add_dimensions(url)
diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index dfe477a8a..a952d0a05 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -5,6 +5,7 @@
 
 defmodule Pleroma.Web.Metadata.Providers.TwitterCard do
   alias Pleroma.User
+  alias Pleroma.Web.MediaProxy
   alias Pleroma.Web.Metadata
   alias Pleroma.Web.Metadata.Providers.Provider
   alias Pleroma.Web.Metadata.Utils
@@ -48,7 +49,8 @@ defp title_tag(user) do
   end
 
   def image_tag(user) do
-    {:meta, [property: "twitter:image", content: Utils.attachment_url(User.avatar_url(user))], []}
+    {:meta, [property: "twitter:image", content: MediaProxy.preview_url(User.avatar_url(user))],
+     []}
   end
 
   defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
@@ -65,13 +67,17 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
                 | acc
               ]
 
+            # Not using preview_url for this. It saves bandwidth, but the image dimensions will be wrong.
+            # We generate it on the fly and have no way to capture or analyze the image to get the dimensions.
+            # This can be an issue for apps/FEs rendering images in timelines too, but you can get clever with
+            # the aspect ratio metadata as a workaround.
             "image" ->
               [
                 {:meta, [property: "twitter:card", content: "summary_large_image"], []},
                 {:meta,
                  [
                    property: "twitter:player",
-                   content: Utils.attachment_url(url["href"])
+                   content: MediaProxy.url(url["href"])
                  ], []}
                 | acc
               ]
@@ -87,7 +93,7 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
                 {:meta, [property: "twitter:player", content: player_url(id)], []},
                 {:meta, [property: "twitter:player:width", content: "#{width}"], []},
                 {:meta, [property: "twitter:player:height", content: "#{height}"], []},
-                {:meta, [property: "twitter:player:stream", content: url["href"]], []},
+                {:meta, [property: "twitter:player:stream", content: MediaProxy.url(url["href"])], []},
                 {:meta,
                  [property: "twitter:player:stream:content_type", content: url["mediaType"]], []}
                 | acc
diff --git a/lib/pleroma/web/metadata/utils.ex b/lib/pleroma/web/metadata/utils.ex
index bc31d66b9..caca42934 100644
--- a/lib/pleroma/web/metadata/utils.ex
+++ b/lib/pleroma/web/metadata/utils.ex
@@ -7,7 +7,6 @@ defmodule Pleroma.Web.Metadata.Utils do
   alias Pleroma.Emoji
   alias Pleroma.Formatter
   alias Pleroma.HTML
-  alias Pleroma.Web.MediaProxy
 
   def scrub_html_and_truncate(%{data: %{"content" => content}} = object) do
     content
@@ -38,10 +37,6 @@ def scrub_html(content) when is_binary(content) do
 
   def scrub_html(content), do: content
 
-  def attachment_url(url) do
-    MediaProxy.preview_url(url)
-  end
-
   def user_name_string(user) do
     "#{user.name} " <>
       if user.local do

From 2cf648d41989dc9cf243fb0972b075726c86adad Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 10:02:41 -0500
Subject: [PATCH 07/15] Add a video thumbnail to the OpenGraph metadata if
 Media Preview Proxy is enabled.

---
 lib/pleroma/web/metadata/providers/open_graph.ex | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index 75d155236..332684782 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -96,6 +96,7 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
                 | acc
               ]
               |> maybe_add_dimensions(url)
+              |> maybe_add_video_thumbnail(url)
 
             _ ->
               acc
@@ -124,4 +125,18 @@ defp maybe_add_dimensions(metadata, url) do
         metadata
     end
   end
+
+  defp maybe_add_video_thumbnail(url, metadata) do
+    cond do
+      Pleroma.Config.get([:media_preview_proxy, :enabled], false) ->
+        [
+          {:meta, [property: "og:image:width", content: "#{url["width"]}"], []},
+          {:meta, [property: "og:image:height", content: "#{url["height"]}"], []},
+          {:meta, [property: "og:image", content: MediaProxy.preview_url(url["href"])], []}
+        ]
+
+      true ->
+        metadata
+    end
+  end
 end

From dc8fe91decd9fd94b5e1ea4fcf2f798430b4c42e Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 10:06:44 -0500
Subject: [PATCH 08/15] Metadata.Utils.attachment_url/1 was used in this test
 too

---
 test/pleroma/web/metadata/providers/twitter_card_test.exs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/pleroma/web/metadata/providers/twitter_card_test.exs b/test/pleroma/web/metadata/providers/twitter_card_test.exs
index dbb15b79f..1b8d27cda 100644
--- a/test/pleroma/web/metadata/providers/twitter_card_test.exs
+++ b/test/pleroma/web/metadata/providers/twitter_card_test.exs
@@ -9,6 +9,7 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCardTest do
   alias Pleroma.User
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.Endpoint
+  alias Pleroma.Web.MediaProxy
   alias Pleroma.Web.Metadata.Providers.TwitterCard
   alias Pleroma.Web.Metadata.Utils
   alias Pleroma.Web.Router
@@ -17,7 +18,7 @@ defmodule Pleroma.Web.Metadata.Providers.TwitterCardTest do
 
   test "it renders twitter card for user info" do
     user = insert(:user, name: "Jimmy Hendriks", bio: "born 19 March 1994")
-    avatar_url = Utils.attachment_url(User.avatar_url(user))
+    avatar_url = MediaProxy.preview_url(User.avatar_url(user))
     res = TwitterCard.build_tags(%{user: user})
 
     assert res == [

From 86bcb87e6c58797387934cbda5ec14b81f3f5f1d Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 11:05:24 -0500
Subject: [PATCH 09/15] Fix incorrectly ordered arguments to the function and
 not properly merging lists.

---
 lib/pleroma/web/metadata/providers/open_graph.ex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index 332684782..0a90904af 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -126,9 +126,10 @@ defp maybe_add_dimensions(metadata, url) do
     end
   end
 
-  defp maybe_add_video_thumbnail(url, metadata) do
+  defp maybe_add_video_thumbnail(metadata, url) do
     cond do
       Pleroma.Config.get([:media_preview_proxy, :enabled], false) ->
+        metadata ++
         [
           {:meta, [property: "og:image:width", content: "#{url["width"]}"], []},
           {:meta, [property: "og:image:height", content: "#{url["height"]}"], []},

From 2a47156b87c668d11f3f2eeee5782472c12c5279 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 11:06:53 -0500
Subject: [PATCH 10/15] Lint

---
 lib/pleroma/web/metadata/providers/open_graph.ex | 16 +++++++++-------
 .../web/metadata/providers/twitter_card.ex       |  3 ++-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index 0a90904af..f6c5c36d7 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -37,7 +37,8 @@ def build_tags(%{
     ] ++
       if attachments == [] or Metadata.activity_nsfw?(object) do
         [
-          {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], []},
+          {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))],
+           []},
           {:meta, [property: "og:image:width", content: 150], []},
           {:meta, [property: "og:image:height", content: 150], []}
         ]
@@ -58,7 +59,8 @@ def build_tags(%{user: user}) do
         {:meta, [property: "og:url", content: user.uri || user.ap_id], []},
         {:meta, [property: "og:description", content: truncated_bio], []},
         {:meta, [property: "og:type", content: "article"], []},
-        {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))], []},
+        {:meta, [property: "og:image", content: MediaProxy.preview_url(User.avatar_url(user))],
+         []},
         {:meta, [property: "og:image:width", content: 150], []},
         {:meta, [property: "og:image:height", content: 150], []}
       ]
@@ -130,11 +132,11 @@ defp maybe_add_video_thumbnail(metadata, url) do
     cond do
       Pleroma.Config.get([:media_preview_proxy, :enabled], false) ->
         metadata ++
-        [
-          {:meta, [property: "og:image:width", content: "#{url["width"]}"], []},
-          {:meta, [property: "og:image:height", content: "#{url["height"]}"], []},
-          {:meta, [property: "og:image", content: MediaProxy.preview_url(url["href"])], []}
-        ]
+          [
+            {:meta, [property: "og:image:width", content: "#{url["width"]}"], []},
+            {:meta, [property: "og:image:height", content: "#{url["height"]}"], []},
+            {:meta, [property: "og:image", content: MediaProxy.preview_url(url["href"])], []}
+          ]
 
       true ->
         metadata
diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index a952d0a05..bfcacf988 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -93,7 +93,8 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
                 {:meta, [property: "twitter:player", content: player_url(id)], []},
                 {:meta, [property: "twitter:player:width", content: "#{width}"], []},
                 {:meta, [property: "twitter:player:height", content: "#{height}"], []},
-                {:meta, [property: "twitter:player:stream", content: MediaProxy.url(url["href"])], []},
+                {:meta, [property: "twitter:player:stream", content: MediaProxy.url(url["href"])],
+                 []},
                 {:meta,
                  [property: "twitter:player:stream:content_type", content: url["mediaType"]], []}
                 | acc

From 5f7901cc48031dc7cb552a63b77721a6457425f6 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 11:09:14 -0500
Subject: [PATCH 11/15] Credo

---
 lib/pleroma/web/metadata/providers/open_graph.ex   | 9 +++++----
 lib/pleroma/web/metadata/providers/twitter_card.ex | 9 +++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index f6c5c36d7..d9f2597ae 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -80,10 +80,11 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
                 | acc
               ]
 
-            # Not using preview_url for this. It saves bandwidth, but the image dimensions will be wrong.
-            # We generate it on the fly and have no way to capture or analyze the image to get the dimensions.
-            # This can be an issue for apps/FEs rendering images in timelines too, but you can get clever with
-            # the aspect ratio metadata as a workaround.
+            # Not using preview_url for this. It saves bandwidth, but the image dimensions will
+            # be wrong. We generate it on the fly and have no way to capture or analyze the
+            # analyze the image to get the dimensions. This can be an issue for apps/FEs
+            # rendering images in timelines too, but you can get clever with the aspect ratio
+            # metadata as a workaround.
             "image" ->
               [
                 {:meta, [property: "og:image", content: MediaProxy.url(url["href"])], []},
diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index bfcacf988..8adab818d 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -67,10 +67,11 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
                 | acc
               ]
 
-            # Not using preview_url for this. It saves bandwidth, but the image dimensions will be wrong.
-            # We generate it on the fly and have no way to capture or analyze the image to get the dimensions.
-            # This can be an issue for apps/FEs rendering images in timelines too, but you can get clever with
-            # the aspect ratio metadata as a workaround.
+            # Not using preview_url for this. It saves bandwidth, but the image dimensions will
+            # be wrong. We generate it on the fly and have no way to capture or analyze the
+            # analyze the image to get the dimensions. This can be an issue for apps/FEs
+            # rendering images in timelines too, but you can get clever with the aspect ratio
+            # metadata as a workaround.
             "image" ->
               [
                 {:meta, [property: "twitter:card", content: "summary_large_image"], []},

From f37db238480f841763555418d11859e3f0a06e5e Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 11:46:31 -0500
Subject: [PATCH 12/15] Test that videos only get image thumbnails in OGP
 metadata when we can produce them with Preview Proxy

---
 .../metadata/providers/open_graph_test.exs    | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/test/pleroma/web/metadata/providers/open_graph_test.exs b/test/pleroma/web/metadata/providers/open_graph_test.exs
index f5f71cee5..28ca8839c 100644
--- a/test/pleroma/web/metadata/providers/open_graph_test.exs
+++ b/test/pleroma/web/metadata/providers/open_graph_test.exs
@@ -107,4 +107,84 @@ test "it does not render attachments if post is nsfw" do
 
     refute {:meta, [property: "og:image", content: "https://misskey.microsoft/corndog.png"], []} in result
   end
+
+  test "video attachments have image thumbnail with WxH metadata with Preview Proxy enabled" do
+    clear_config([:media_proxy, :enabled], true)
+    clear_config([:media_preview_proxy, :enabled], true)
+    user = insert(:user)
+
+    note =
+      insert(:note, %{
+        data: %{
+          "actor" => user.ap_id,
+          "id" => "https://pleroma.gov/objects/whatever",
+          "content" => "test video post",
+          "sensitive" => false,
+          "attachment" => [
+            %{
+              "url" => [
+                %{
+                  "mediaType" => "video/webm",
+                  "href" => "https://pleroma.gov/about/juche.webm",
+                  "height" => 600,
+                  "width" => 800
+                }
+              ]
+            }
+          ]
+        }
+      })
+
+    result = OpenGraph.build_tags(%{object: note, url: note.data["id"], user: user})
+
+    assert {:meta, [property: "og:image:width", content: "800"], []} in result
+    assert {:meta, [property: "og:image:height", content: "600"], []} in result
+
+    assert {:meta,
+            [
+              property: "og:image",
+              content:
+                "http://localhost:4001/proxy/preview/LzAnlke-l5oZbNzWsrHfprX1rGw/aHR0cHM6Ly9wbGVyb21hLmdvdi9hYm91dC9qdWNoZS53ZWJt/juche.webm"
+            ], []} in result
+  end
+
+  test "video attachments have no image thumbnail with Preview Proxy disabled" do
+    clear_config([:media_proxy, :enabled], true)
+    clear_config([:media_preview_proxy, :enabled], false)
+    user = insert(:user)
+
+    note =
+      insert(:note, %{
+        data: %{
+          "actor" => user.ap_id,
+          "id" => "https://pleroma.gov/objects/whatever",
+          "content" => "test video post",
+          "sensitive" => false,
+          "attachment" => [
+            %{
+              "url" => [
+                %{
+                  "mediaType" => "video/webm",
+                  "href" => "https://pleroma.gov/about/juche.webm",
+                  "height" => 600,
+                  "width" => 800
+                }
+              ]
+            }
+          ]
+        }
+      })
+
+    result = OpenGraph.build_tags(%{object: note, url: note.data["id"], user: user})
+
+    refute {:meta, [property: "og:image:width", content: "800"], []} in result
+    refute {:meta, [property: "og:image:height", content: "600"], []} in result
+
+    refute {:meta,
+            [
+              property: "og:image",
+              content:
+                "http://localhost:4001/proxy/preview/LzAnlke-l5oZbNzWsrHfprX1rGw/aHR0cHM6Ly9wbGVyb21hLmdvdi9hYm91dC9qdWNoZS53ZWJt/juche.webm"
+            ], []} in result
+  end
 end

From d12e62c0b6f83a439c49a4bd94b4e77e53da66a1 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 11:56:54 -0500
Subject: [PATCH 13/15] Add new Twittercard/OGP changes

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dcb462f07..52d92c6d2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - The `application` metadata returned with statuses is no longer hardcoded. Apps that want to display these details will now have valid data for new posts after this change.
 - HTTPSecurityPlug now sends a response header to opt out of Google's FLoC (Federated Learning of Cohorts) targeted advertising.
 - Email address is now returned if requesting user is the owner of the user account so it can be exposed in client and FE user settings UIs.
+- Improved Twittercard and OpenGraph meta tag generation including thumbnails and image dimension metadata when available.
 
 ### Added
 

From 6aa7fc15df372478fbff02730bc521fab2ccd1e3 Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Wed, 9 Jun 2021 11:58:51 -0500
Subject: [PATCH 14/15] Formatting of the comment

---
 lib/pleroma/web/metadata/providers/open_graph.ex   | 6 +++---
 lib/pleroma/web/metadata/providers/twitter_card.ex | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index d9f2597ae..ef4ad6885 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -82,9 +82,9 @@ defp build_attachments(%{data: %{"attachment" => attachments}}) do
 
             # Not using preview_url for this. It saves bandwidth, but the image dimensions will
             # be wrong. We generate it on the fly and have no way to capture or analyze the
-            # analyze the image to get the dimensions. This can be an issue for apps/FEs
-            # rendering images in timelines too, but you can get clever with the aspect ratio
-            # metadata as a workaround.
+            # image to get the dimensions. This can be an issue for apps/FEs rendering images
+            # in timelines too, but you can get clever with the aspect ratio metadata as a
+            # workaround.
             "image" ->
               [
                 {:meta, [property: "og:image", content: MediaProxy.url(url["href"])], []},
diff --git a/lib/pleroma/web/metadata/providers/twitter_card.ex b/lib/pleroma/web/metadata/providers/twitter_card.ex
index 8adab818d..79183df86 100644
--- a/lib/pleroma/web/metadata/providers/twitter_card.ex
+++ b/lib/pleroma/web/metadata/providers/twitter_card.ex
@@ -69,9 +69,9 @@ defp build_attachments(id, %{data: %{"attachment" => attachments}}) do
 
             # Not using preview_url for this. It saves bandwidth, but the image dimensions will
             # be wrong. We generate it on the fly and have no way to capture or analyze the
-            # analyze the image to get the dimensions. This can be an issue for apps/FEs
-            # rendering images in timelines too, but you can get clever with the aspect ratio
-            # metadata as a workaround.
+            # image to get the dimensions. This can be an issue for apps/FEs rendering images
+            # in timelines too, but you can get clever with the aspect ratio metadata as a
+            # workaround.
             "image" ->
               [
                 {:meta, [property: "twitter:card", content: "summary_large_image"], []},

From 202ee5fd77e721c8822dd779c8b558ec8cfacfcc Mon Sep 17 00:00:00 2001
From: Mark Felder <feld@feld.me>
Date: Thu, 10 Jun 2021 09:56:43 -0500
Subject: [PATCH 15/15] Add note about video thumbnails for code spelunkers
 unfamiliar with Media Preview Proxy

---
 lib/pleroma/web/metadata/providers/open_graph.ex | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/pleroma/web/metadata/providers/open_graph.ex b/lib/pleroma/web/metadata/providers/open_graph.ex
index ef4ad6885..df0cca74a 100644
--- a/lib/pleroma/web/metadata/providers/open_graph.ex
+++ b/lib/pleroma/web/metadata/providers/open_graph.ex
@@ -129,6 +129,8 @@ defp maybe_add_dimensions(metadata, url) do
     end
   end
 
+  # Media Preview Proxy makes thumbnails of videos without resizing, so we can trust the
+  # width and height of the source video.
   defp maybe_add_video_thumbnail(metadata, url) do
     cond do
       Pleroma.Config.get([:media_preview_proxy, :enabled], false) ->