From 37e2a35b865e395ef635bfd6cfb0cb7bca985048 Mon Sep 17 00:00:00 2001 From: Oneric Date: Fri, 16 Feb 2024 04:32:09 +0100 Subject: [PATCH 1/2] Fix Twitter metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This partly reverts 1d884fd9143dc165d745bf1b5e86bcc65332d6b9 while fixing both the issue it addressed and the issue it caused. The above commit successfully fixed OpenGraph metadata tags which until then always showed the user bio instead of post content by handing the activities AP ID as url to the Metadata builder _instead_ of passing the internal ID as activity_id. However, in doing so the commit instead inflicted this very problem onto Twitter metadata tags which ironically are used by akkoma-fe. This is because while the OpenGraph builder wants an URL as url, the Twitter builder needs the internal ID to build the URL to the embedded player for videos and has no URL property. Thanks to twpol for tracking down this root cause in #644. Now, once identified the problem is simple, but this simplicity invites multiple possible solutions to bikeshed about. 1. Just pass both properties to the builder and let them pick 2. Drop the url parameter from the OpenGraph builder and instead a) build static-fe URL of the post from the ID (like Twitter) b) use the passed-in object’s AP ID as an URL Approach 2a has the disadvantage of hardcoding the expected URL outside the router, which will be problematic should it ever change. Approach 2b is conceptually similar to how the builder works atm. However, the og:url is supposed to be a _permanent_ ID, by changing it we might, afaiui, technically violate OpenGraph specs(?). (Though its real-world consequence may very well be near non-existent.) This leaves just approach 1, which this commit implements. Albeit it too is not without nits to pick, as it leaves the metadata builders with an inconsistent interface. Additionally, this will resolve the subotpimal Discord previews for content-less image posts reported in #664. Discord already prefers OpenGraph metadata, so it’s mostly unaffected. However, it appears when encountering an explicitly empty OpenGraph description and a non-empty Twitter description, it replaces just the empty field with its Twitter counterpart, resulting in the user’s bio slipping into the preview. Secondly, regardless of any OpenGraph tags, Discord uses twitter:card to decide how prominently images should be, but due to the bug the card type was stuck as "summary", forcing images to always remain small. Root cause identified by: twpol Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/644 Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/664 --- lib/pleroma/web/static_fe/static_fe_controller.ex | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/static_fe/static_fe_controller.ex b/lib/pleroma/web/static_fe/static_fe_controller.ex index f0d45293e..b1ea3178d 100644 --- a/lib/pleroma/web/static_fe/static_fe_controller.ex +++ b/lib/pleroma/web/static_fe/static_fe_controller.ex @@ -24,7 +24,13 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do true <- Visibility.is_public?(activity.object), {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)}, %User{} = user <- User.get_by_ap_id(activity.object.data["actor"]) do - meta = Metadata.build_tags(%{url: activity.data["id"], object: activity.object, user: user}) + meta = + Metadata.build_tags(%{ + activity_id: notice_id, + url: activity.data["id"], + object: activity.object, + user: user + }) timeline = activity.object.data["context"] From c08f49d88ebbef62cdfa4bf4004eac236c0e42af Mon Sep 17 00:00:00 2001 From: Oneric Date: Wed, 21 Feb 2024 00:33:32 +0000 Subject: [PATCH 2/2] Add tests for static-fe metadata tags --- .../static_fe/static_fe_controller_test.exs | 159 +++++++++++++++++- 1 file changed, 157 insertions(+), 2 deletions(-) diff --git a/test/pleroma/web/static_fe/static_fe_controller_test.exs b/test/pleroma/web/static_fe/static_fe_controller_test.exs index 935e44171..79d4d6261 100644 --- a/test/pleroma/web/static_fe/static_fe_controller_test.exs +++ b/test/pleroma/web/static_fe/static_fe_controller_test.exs @@ -19,9 +19,26 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do setup %{conn: conn} do conn = put_req_header(conn, "accept", "text/html") - user = insert(:user) - %{conn: conn, user: user} + user_avatar_url = "https://example.org/akko.png" + + user = + insert(:user, + local: true, + name: "Akko", + nickname: "atsuko", + bio: "A believing heart is my magic!", + raw_bio: "A believing heart is my magic!", + avatar: %{ + "url" => [ + %{ + "href" => user_avatar_url + } + ] + } + ) + + %{conn: conn, user: user, user_avatar_url: user_avatar_url} end describe "user profile html" do @@ -291,4 +308,142 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do |> html_response(404) end end + + defp meta_content(metadata_tag) do + :proplists.get_value("content", metadata_tag) + end + + defp meta_find_og(document, name) do + Floki.find(document, "head>meta[property=\"og:" <> name <> "\"]") + end + + defp meta_find_twitter(document, name) do + Floki.find(document, "head>meta[name=\"twitter:" <> name <> "\"]") + end + + # Detailed metadata tests are already done for each builder individually, so just + # one check per type of content should suffice to ensure we're calling the providers correctly + describe "metadata tags for" do + setup do + clear_config([Pleroma.Web.Metadata, :providers], [ + Pleroma.Web.Metadata.Providers.OpenGraph, + Pleroma.Web.Metadata.Providers.TwitterCard + ]) + end + + test "user profile", %{conn: conn, user: user, user_avatar_url: user_avatar_url} do + conn = get(conn, "/users/#{user.nickname}") + html = html_response(conn, 200) + + {:ok, document} = Floki.parse_document(html) + + [{"meta", og_type, _}] = meta_find_og(document, "type") + [{"meta", og_title, _}] = meta_find_og(document, "title") + [{"meta", og_url, _}] = meta_find_og(document, "url") + [{"meta", og_desc, _}] = meta_find_og(document, "description") + [{"meta", og_img, _}] = meta_find_og(document, "image") + [{"meta", og_imgw, _}] = meta_find_og(document, "image:width") + [{"meta", og_imgh, _}] = meta_find_og(document, "image:height") + + [{"meta", tw_card, _}] = meta_find_twitter(document, "card") + [{"meta", tw_title, _}] = meta_find_twitter(document, "title") + [{"meta", tw_desc, _}] = meta_find_twitter(document, "description") + [{"meta", tw_img, _}] = meta_find_twitter(document, "image") + + assert meta_content(og_type) == "article" + assert meta_content(og_title) == Pleroma.Web.Metadata.Utils.user_name_string(user) + assert meta_content(og_url) == user.ap_id + assert meta_content(og_desc) == user.bio + assert meta_content(og_img) == user_avatar_url + assert meta_content(og_imgw) == "150" + assert meta_content(og_imgh) == "150" + + assert meta_content(tw_card) == "summary" + assert meta_content(tw_title) == meta_content(og_title) + assert meta_content(tw_desc) == meta_content(og_desc) + assert meta_content(tw_img) == meta_content(og_img) + end + + test "text-only post", %{conn: conn, user: user, user_avatar_url: user_avatar_url} do + post_text = "How are lessons about magic t h i s boring?!" + {:ok, activity} = CommonAPI.post(user, %{status: post_text}) + + conn = get(conn, "/notice/#{activity.id}") + html = html_response(conn, 200) + + {:ok, document} = Floki.parse_document(html) + + [{"meta", og_type, _}] = meta_find_og(document, "type") + [{"meta", og_title, _}] = meta_find_og(document, "title") + [{"meta", og_url, _}] = meta_find_og(document, "url") + [{"meta", og_desc, _}] = meta_find_og(document, "description") + [{"meta", og_img, _}] = meta_find_og(document, "image") + [{"meta", og_imgw, _}] = meta_find_og(document, "image:width") + [{"meta", og_imgh, _}] = meta_find_og(document, "image:height") + + [{"meta", tw_card, _}] = meta_find_twitter(document, "card") + [{"meta", tw_title, _}] = meta_find_twitter(document, "title") + [{"meta", tw_desc, _}] = meta_find_twitter(document, "description") + [{"meta", tw_img, _}] = meta_find_twitter(document, "image") + + assert meta_content(og_type) == "article" + assert meta_content(og_title) == Pleroma.Web.Metadata.Utils.user_name_string(user) + assert meta_content(og_url) == activity.data["id"] + assert meta_content(og_desc) == post_text + assert meta_content(og_img) == user_avatar_url + assert meta_content(og_imgw) == "150" + assert meta_content(og_imgh) == "150" + + assert meta_content(tw_card) == "summary" + assert meta_content(tw_title) == meta_content(og_title) + assert meta_content(tw_desc) == meta_content(og_desc) + assert meta_content(tw_img) == meta_content(og_img) + end + + test "post with attachments", %{conn: conn, user: user} do + file = %Plug.Upload{ + content_type: "image/jpeg", + path: Path.absname("test/fixtures/image.jpg"), + filename: "an_image.jpg" + } + + alt_text = "The rarest of all Shiny Chariot cards" + {:ok, upload_data} = ActivityPub.upload(file, actor: user.ap_id, description: alt_text) + + %{id: media_id, data: %{"url" => [%{"href" => media_url}]}} = upload_data + + post_text = "Look!" + {:ok, activity} = CommonAPI.post(user, %{status: post_text, media_ids: [media_id]}) + + conn = get(conn, "/notice/#{activity.id}") + html = html_response(conn, 200) + + {:ok, document} = Floki.parse_document(html) + + [{"meta", og_type, _}] = meta_find_og(document, "type") + [{"meta", og_title, _}] = meta_find_og(document, "title") + [{"meta", og_url, _}] = meta_find_og(document, "url") + [{"meta", og_desc, _}] = meta_find_og(document, "description") + [{"meta", og_img, _}] = meta_find_og(document, "image") + [{"meta", og_alt, _}] = meta_find_og(document, "image:alt") + + [{"meta", tw_card, _}] = meta_find_twitter(document, "card") + [{"meta", tw_title, _}] = meta_find_twitter(document, "title") + [{"meta", tw_desc, _}] = meta_find_twitter(document, "description") + [{"meta", tw_player, _}] = meta_find_twitter(document, "player") + + assert meta_content(og_type) == "article" + assert meta_content(og_title) == Pleroma.Web.Metadata.Utils.user_name_string(user) + assert meta_content(og_url) == activity.data["id"] + assert meta_content(og_desc) == post_text + assert meta_content(og_img) == media_url + assert meta_content(og_alt) == alt_text + + # Audio and video attachments use "player" and have some more metadata + assert meta_content(tw_card) == "summary_large_image" + assert meta_content(tw_title) == meta_content(og_title) + assert meta_content(tw_desc) == meta_content(og_desc) + assert meta_content(tw_player) == meta_content(og_img) + end + end end