Federate user profile background #682

Merged
floatingghost merged 2 commits from Oneric/akkoma:background-federation into develop 2024-02-16 21:00:11 +00:00
Member

The property existed for ages and was publicly visible via API, but not federate nor shown to others in our own frontend.

Recently Sharkey added support for profile backgrounds and immediately made them federate and be displayed to others. This uses the same AP field as Sharkey here which should make it interoperable both ways out-of-the-box.
Ref.: Sharkey@4e643976

I have no frontend patch (yet), but even so this is still useful as it will start to gather remote background URLs so more stuff is visible once the FE is patched and already immediately should allow Sharkey users to see backgrounds of Akkoma users (albeit I only tested the Sharkey→Akkoma direction in the real world).

Also it turns out there’s an oldfeature request for this, so this will also fix #290

The property existed for ages and was publicly visible via API, but not federate nor shown to others in our own frontend. Recently Sharkey added support for profile backgrounds and immediately made them federate and be displayed to others. This uses the same AP field as Sharkey here which should make it interoperable both ways out-of-the-box. Ref.: [`Sharkey@4e643976`](https://activitypub.software/TransFem-org/Sharkey/-/commit/4e6439763544f7b96009dd1411035343fb561d2a) I have no frontend patch (yet), but even so this is still useful as it will start to gather remote background URLs so more stuff is visible once the FE is patched and already immediately *should* allow Sharkey users to see backgrounds of Akkoma users *(albeit I only tested the Sharkey→Akkoma direction in the real world)*. Also it turns out there’s an oldfeature request for this, so this will also fix https://akkoma.dev/AkkomaGang/akkoma/issues/290

patch looks ok at first glance - but this will absolutely need a new preference to not show other people's backgrounds (and probably amending the MRF simple to strip backgrounds from some instances) to avoid abuse of the field

patch looks ok at first glance - but this will absolutely need a new preference to not show other people's backgrounds (and probably amending the MRF simple to strip backgrounds from some instances) to avoid abuse of the field
Oneric force-pushed background-federation from 891b99d6f1 to 4ff7cc9c95 2024-02-10 16:53:13 +00:00 Compare
Author
Member

this will absolutely need a new preference to not show other people's backgrounds

yep, adding a frontend config (once the FE can show it at all) was the plan

and probably amending the MRF simple to strip backgrounds from some instances

oh yeah, good call. I was at first thinking it might make sense to tie this to media_removal and (depending on a boolean toggle) media_nsfw — but then I noticed avatars and banners already have dedicated config lists each and it’d be weird to only tie backgrounds to something else.

Pushed a SimplePolicy extension which basically just copy-pastas the existing avatar and banner logic for backgroundUrl.
(And added Changelog entries to both commits)

EDIT: ofc I forgot mix format again °~° — oh well its applied now

> this will absolutely need a new preference to not show other people's backgrounds yep, adding a frontend config (once the FE can show it at all) was the plan > and probably amending the MRF simple to strip backgrounds from some instances oh yeah, good call. I was at first thinking it might make sense to tie this to `media_removal` and (depending on a boolean toggle) `media_nsfw` — but then I noticed avatars and banners already have dedicated config lists each and it’d be weird to only tie backgrounds to something else. Pushed a SimplePolicy extension which basically just copy-pastas the existing avatar and banner logic for `backgroundUrl`. (And added Changelog entries to both commits) **EDIT:** ofc I forgot `mix format` again °~° — oh well its applied now
Oneric force-pushed background-federation from 4ff7cc9c95 to 4c223ac5a1 2024-02-10 17:04:19 +00:00 Compare
Oneric force-pushed background-federation from 4c223ac5a1 to 219d2d8893 2024-02-11 03:35:09 +00:00 Compare
floatingghost reviewed 2024-02-16 13:33:19 +00:00
@ -112,6 +112,8 @@ defmodule Pleroma.Web.ActivityPub.UserView do
}
|> Map.merge(maybe_make_image(&User.avatar_url/2, "icon", user))
|> Map.merge(maybe_make_image(&User.banner_url/2, "image", user))
# Yes, the key is named ...Url eventhough it is a whole 'Image' object

hmmm, on closer inspection maybe we can do something about this

we've had situations like this before when dealing with quoteUrl - where we use a more coherent internal key and rewrite when other people throw us weird names like this

i guess you've used this for compatibility, which makes it hard.

i'll let it slide for now since we use a decent database key for it, but something to bear in mind

hmmm, on closer inspection maybe we can do something about this we've had situations like this before when [dealing with quoteUrl](https://akkoma.dev/AkkomaGang/akkoma/src/branch/develop/lib/pleroma/web/activity_pub/transmogrifier.ex#L151) - where we use a more coherent internal key and rewrite when other people throw us weird names like this i guess you've used this for compatibility, which makes it hard. i'll let it slide for now since we use a decent database key for it, but something to bear in mind
Author
Member

yeah, if we use a different name in the Activity Pub view, Sharkey instances won’t be able to read backgrounds from Akkoma.

Since afaik Sharkey is the only other implementation, I guess if you want I could ask on the Sharkey tracker whether they’re willing to deal with a property rename and its added work

But also:

a more coherent internal key

iinm unlike most other objects users don’t get their whole AP object shoved into the database, so user.background already is the only internal representation and backgroundUrl is only used when processing/creating AP objects from/for remote instances

Creating and parsing the field with a more sensible name and rewriting its name for both in- and outgoing messages in transmogrifier.ex imho doesn’t make much sense.


(force-pushed to rebase and resolve the changelog conflict)

yeah, if we use a different name in the Activity Pub view, Sharkey instances won’t be able to read backgrounds from Akkoma. Since afaik Sharkey is the only other implementation, I guess if you want I could ask on the Sharkey tracker whether they’re willing to deal with a property rename and its added work But also: > a more coherent internal key iinm unlike most other objects users don’t get their whole AP object shoved into the database, so `user.background` already is the only *internal* representation and `backgroundUrl` is only used when processing/creating AP objects from/for remote instances Creating and parsing the field with a more sensible name and rewriting its name for both in- and outgoing messages in `transmogrifier.ex` imho doesn’t make much sense. -------- *(force-pushed to rebase and resolve the changelog conflict)*
Oneric force-pushed background-federation from 219d2d8893 to e99e2407f3 2024-02-16 15:44:25 +00:00 Compare

yea seems good to me, let's keep an eye on if anyone else implements this, and if they do maybe we can go for standardisation

for now it's ok, thanks!

yea seems good to me, let's keep an eye on if anyone else implements this, and if they do maybe we can go for standardisation for now it's ok, thanks!
floatingghost merged commit 34c213f02f into develop 2024-02-16 21:00:11 +00:00
floatingghost deleted branch background-federation 2024-02-16 21:00:11 +00:00
Sign in to join this conversation.
No description provided.