New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add semi-support for Video/Image objects in ActivityPub #5848

Merged
merged 3 commits into from Nov 30, 2017

Conversation

Projects
None yet
4 participants
@Gargron
Member

Gargron commented Nov 29, 2017

PeerTube now talks ActivityPub: Chocobozzz/PeerTube#104

Video and Image objects will create corresponding text status records with manually crafted text contents (title + URL). Furthermore, the code now supports extracting the language from nameMap attribute as fallback of contentMap (name = title, since there is no title attribute)

The implications of this is that since PeerTube implements the right OpenGraph tags (e.g. twitter:player) such toots will actually embed the PeerTube video player as their preview card; it also means that replies to the "toot" will still be sent to the right object upstream and show up as comments on PeerTube.

Mind that at the time of writing PeerTube actually needs to fix their language serialization because they use a non-existing language attribute when the established way is nameMap/contentMap, and likewise the text/html URL is missing from their url array.

Add semi-support for Video/Image objects in ActivityPub
Video and Image objects will create corresponding status records
with manually crafted text contents (title + URL)
elsif name_language_map?
@object['nameMap'].values.first
end
end

This comment has been minimized.

@nightpool

nightpool Nov 29, 2017

Collaborator

can be @object['name] || @object['nameMap']&.values&.first but it feels like we need a helper here instead, since every text value (name, content, preferredUsername, tags) can have this form and we have helpers for similar JSON-LD conventions (equals_or_includes, value_or_id)

@nightpool

nightpool Nov 29, 2017

Collaborator

can be @object['name] || @object['nameMap']&.values&.first but it feels like we need a helper here instead, since every text value (name, content, preferredUsername, tags) can have this form and we have helpers for similar JSON-LD conventions (equals_or_includes, value_or_id)

This comment has been minimized.

@Gargron

Gargron Nov 29, 2017

Member

This only comes up twice so I disagree about the need to abstract it.

@Gargron

Gargron Nov 29, 2017

Member

This only comes up twice so I disagree about the need to abstract it.

This comment has been minimized.

@nightpool

nightpool Nov 29, 2017

Collaborator

It's not because of DRY reasons that I'm saying it should be abstracted, it's because it's a fundamental part of processing JSON-LD content that can be implemented/tested/used independently of any actual activity processing.

@nightpool

nightpool Nov 29, 2017

Collaborator

It's not because of DRY reasons that I'm saying it should be abstracted, it's because it's a fundamental part of processing JSON-LD content that can be implemented/tested/used independently of any actual activity processing.

@object['contentMap'].keys.first
elsif name_language_map?
@object['nameMap'].keys.first
elsif supported_object_type?

This comment has been minimized.

@nightpool

nightpool Nov 29, 2017

Collaborator

why not do this for converted object types?

@nightpool

nightpool Nov 29, 2017

Collaborator

why not do this for converted object types?

This comment has been minimized.

@Gargron

Gargron Nov 29, 2017

Member

If a Video does not have a language attribute, what's the point of trying to detect language from its description? What would language even refer to if we're talking about a Video or an Image? for text posts it's clear. For media, it is not, because the thing itself is not the text, the text is merely a description, which may be missing, which may be in a different language.

@Gargron

Gargron Nov 29, 2017

Member

If a Video does not have a language attribute, what's the point of trying to detect language from its description? What would language even refer to if we're talking about a Video or an Image? for text posts it's clear. For media, it is not, because the thing itself is not the text, the text is merely a description, which may be missing, which may be in a different language.

This comment has been minimized.

@nightpool

nightpool Nov 29, 2017

Collaborator

There may be other converted object types, like if we wanted to implement base Activities, but I see your point.

@nightpool

nightpool Nov 29, 2017

Collaborator

There may be other converted object types, like if we wanted to implement base Activities, but I see your point.

This comment has been minimized.

@Gargron

Gargron Nov 29, 2017

Member

There may be other converted object types

AP doesn't have a general language attribute. It can only provide languages for name/content. That means it can only be relevant for text posts where object == its text.

@Gargron

Gargron Nov 29, 2017

Member

There may be other converted object types

AP doesn't have a general language attribute. It can only provide languages for name/content. That means it can only be relevant for text posts where object == its text.

@@ -69,6 +64,14 @@ def format_spoiler(status)
html.html_safe # rubocop:disable Rails/OutputSafety
end
def linkify(text)
html = encode_and_link_urls(text)

This comment has been minimized.

@nightpool

nightpool Nov 29, 2017

Collaborator

does encode_and_link_urls sanitize correctly here? Or am I missing where the HTML-safety happens?

@nightpool

nightpool Nov 29, 2017

Collaborator

does encode_and_link_urls sanitize correctly here? Or am I missing where the HTML-safety happens?

This comment has been minimized.

@Gargron

Gargron Nov 29, 2017

Member

Yes, it does that. This function has been extracted verbatim from simplified_format.

@Gargron

Gargron Nov 29, 2017

Member

Yes, it does that. This function has been extracted verbatim from simplified_format.

This comment has been minimized.

@nightpool

nightpool Nov 29, 2017

Collaborator

@nightpool

nightpool Nov 29, 2017

Collaborator

@nightpool

SHOULD:

  • remove hard-coded types in fetch_remote_status_service
  • fall back to id if object['url'] isn't included

MAY:

  • extract the complicated link-value logic into another function for readability
@nightpool

LGTM!

@Gargron Gargron merged commit 4c6b5db into master Nov 30, 2017

3 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@Gargron Gargron deleted the feature-converted-activitypub-types branch Nov 30, 2017

@Chocobozzz

This comment has been minimized.

Show comment
Hide comment
@Chocobozzz

Chocobozzz Nov 30, 2017

Hi,

The custom language in PeerTube is in fact the video's language. The language of the name/content could be different and is unknown in PeerTube.

We added the text/html url in the object -> Chocobozzz/PeerTube@165cdc7#diff-852ba6d236e543da3d7c6d5128f06759R619

The last incompatibility is the signature algorithm, we need to implement RsaSignature2017. I asked the devs of the module we use if there are any news and I'm waiting for their response. If nothing move in the next weeks I'll implement it myself :)

Chocobozzz commented Nov 30, 2017

Hi,

The custom language in PeerTube is in fact the video's language. The language of the name/content could be different and is unknown in PeerTube.

We added the text/html url in the object -> Chocobozzz/PeerTube@165cdc7#diff-852ba6d236e543da3d7c6d5128f06759R619

The last incompatibility is the signature algorithm, we need to implement RsaSignature2017. I asked the devs of the module we use if there are any news and I'm waiting for their response. If nothing move in the next weeks I'll implement it myself :)

@Gargron

This comment has been minimized.

Show comment
Hide comment
@Gargron

Gargron Nov 30, 2017

Member

@Chocobozzz ld-sigs are only relevant for forwarding which in case of peertube doesn't seem like a big deal to me. like, if i replied to a video of yours, you'd want to forward my reply to all people you know who have that video too; but they won't notice if you don't do that. the other use case is when you delete a video, you'd want people who have reblogged it to also forward the delete to their followers so it disappears from all the places it propagated.

Member

Gargron commented Nov 30, 2017

@Chocobozzz ld-sigs are only relevant for forwarding which in case of peertube doesn't seem like a big deal to me. like, if i replied to a video of yours, you'd want to forward my reply to all people you know who have that video too; but they won't notice if you don't do that. the other use case is when you delete a video, you'd want people who have reblogged it to also forward the delete to their followers so it disappears from all the places it propagated.

cobodo pushed a commit to cobodo/mastodon that referenced this pull request Dec 6, 2017

Add semi-support for Video/Image objects in ActivityPub (tootsuite#5848)
* Add semi-support for Video/Image objects in ActivityPub

Video and Image objects will create corresponding status records
with manually crafted text contents (title + URL)

* Extract html-url-finding logic into JsonLdHelper

* Fallback to id when url missing, extract supported object types
@drequivalent

This comment has been minimized.

Show comment
Hide comment
@drequivalent

drequivalent Dec 18, 2017

Can't follow chocobozzz@peertube.cpy from my 2.1.0 instance
screenshot from 2017-12-18 03-46-49
screenshot from 2017-12-18 03-46-58

drequivalent commented Dec 18, 2017

Can't follow chocobozzz@peertube.cpy from my 2.1.0 instance
screenshot from 2017-12-18 03-46-49
screenshot from 2017-12-18 03-46-58

@Chocobozzz

This comment has been minimized.

Show comment
Hide comment
@Chocobozzz

Chocobozzz Dec 18, 2017

It's chocobozzz@peertube.cpy.re:443 but it won't really work, it's still a WIP on PeerTube. I'm currently working on this.

Chocobozzz commented Dec 18, 2017

It's chocobozzz@peertube.cpy.re:443 but it won't really work, it's still a WIP on PeerTube. I'm currently working on this.

@drequivalent

This comment has been minimized.

Show comment
Hide comment
@drequivalent

drequivalent commented Dec 18, 2017

Understood.

kaniini added a commit to kaniini/mastodon that referenced this pull request Mar 9, 2018

Add semi-support for Video/Image objects in ActivityPub (tootsuite#5848)
* Add semi-support for Video/Image objects in ActivityPub

Video and Image objects will create corresponding status records
with manually crafted text contents (title + URL)

* Extract html-url-finding logic into JsonLdHelper

* Fallback to id when url missing, extract supported object types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment