LemmyNet
/
lemmy
3
2
Fork 3
Code Pull Requests 1 Releases Wiki Activity

Populate content with HTML, and source with markdown (ref #1220) #141

Merged
dessalines merged 4 commits from apub-media-type2 into main 2020-11-25 19:54:54 +00:00
Conversation 6 Commits 4 Files Changed 20 +341 -120
nutomic commented 2020-11-24 17:54:31 +00:00
Owner
Copy Link

Here we go, doing it the proper way.

Here we go, doing it the proper way.
nutomic reviewed 2020-11-24 17:55:44 +00:00
lemmy_apub/src/objects/user.rs Outdated
@ -49,2 +55,4 @@
if let Some(bio) = &self.bio {
set_content_and_source(&mut person, bio)?;
// Also set summary for compatibility with older Lemmy versions. Remove this after a while.
person.set_summary(bio.to_owned());
nutomic commented 2020-11-24 17:55:44 +00:00
Author
Owner
Copy Link

I had to change the user bio from summary to content, because the spec says that summary can only be HTML, with no option to specify a different mime type.

I had to change the user bio from `summary` to `content`, because the spec says that summary can only be HTML, with no option to specify a different mime type.
dessalines commented 2020-11-24 21:06:58 +00:00
Owner
Copy Link

I'm re-running this on travis right now, you needed to merge from main because the builds are broken otherwise.

I'm re-running this on travis right now, you needed to merge from main because the builds are broken otherwise.
dessalines reviewed 2020-11-24 21:12:02 +00:00
lemmy_apub/src/objects/mod.rs Outdated
@ -61,0 +76,4 @@
.set_content(markdown_text)
.set_media_type(mime_markdown()?);
object.set_source(source.into_any_base()?);
object.set_content(markdown_to_html(markdown_text));
dessalines commented 2020-11-24 21:12:02 +00:00
Owner
Copy Link

This seems really dangerous, why do we need to convert to html? Why not just content: mardownText, type: 'markdown'

This seems really dangerous, why do we need to convert to html? Why not just content: mardownText, type: 'markdown'
nutomic commented 2020-11-25 11:58:03 +00:00
Author
Owner
Copy Link

We could also put Markdown into the top-level content and set mediaType to text/markdown. But doing it this way ensures compatibility with software which doesnt support Markdown. Anyway, Lemmy doesnt read the HTML, only Markdown from source.content, so this doesnt change anything.

Except, now that I'm thinking about it, federation with instances without and with this commit will be broken.

We could also put Markdown into the top-level `content` and set [mediaType](https://www.w3.org/TR/activitystreams-vocabulary/#dfn-mediatype) to `text/markdown`. But doing it this way ensures compatibility with software which doesnt support Markdown. Anyway, Lemmy doesnt read the HTML, only Markdown from `source.content`, so this doesnt change anything. Except, now that I'm thinking about it, federation with instances without and with this commit will be broken.
nutomic commented 2020-11-25 12:07:51 +00:00
Author
Owner
Copy Link

I suppose for compatibility we could change both content and source.content to Markdown, then later change content to HTML. But that gets quite complicated really.

I suppose for compatibility we could change both `content` and `source.content` to Markdown, then later change `content` to HTML. But that gets quite complicated really.
dessalines reviewed 2020-11-24 21:15:53 +00:00
lemmy_apub/src/objects/user.rs Outdated
@ -125,3 +128,1 @@
.map(|s| s.as_single_xsd_string())
.flatten()
.map(|s| s.to_string());
let bio = get_source_markdown_value(person)?;
dessalines commented 2020-11-24 21:15:53 +00:00
Owner
Copy Link

Seems to be good, but we'll have to test manually before we merge.

Seems to be good, but we'll have to test manually before we merge.
nutomic changed title from Populate `content` with HTML, and `source` with markdown (ref #1220) to WIP: Populate `content` with HTML, and `source` with markdown (ref #1220) 2020-11-25 12:42:31 +00:00
nutomic changed title from WIP: Populate `content` with HTML, and `source` with markdown (ref #1220) to Populate `content` with HTML, and `source` with markdown (ref #1220) 2020-11-25 13:08:04 +00:00
nutomic commented 2020-11-25 13:09:41 +00:00
Author
Owner
Copy Link

I added a commit to make this change backwards compatible, meaning both content and source.content contain Markdown for now. We will have to revert that in a while for maximum compatibility.

I tested by running that new commit on ds9, and it federates fine with voyager.

https://ds9.lemmy.ml/post/137

https://voyager.lemmy.ml/post/120

Note that the documentation is wrong now, cause I dont want to change that back and forth.

I added a commit to make this change backwards compatible, meaning both `content` and `source.content` contain Markdown for now. We will have to revert that in a while for maximum compatibility. I tested by running that new commit on ds9, and it federates fine with voyager. https://ds9.lemmy.ml/post/137 https://voyager.lemmy.ml/post/120 Note that the documentation is wrong now, cause I dont want to change that back and forth.
dessalines commented 2020-11-25 19:54:46 +00:00
Owner
Copy Link

Seems good. About the html stuff (we can talk about this later on), I assumed it was up to the receiving server to decide what to do with the parsed info:

IE you receive `{content: "some italicized text", type: "text/markdown"}, so that server has to get that type and convert it to whatever their DB / front end representation requires.

Seems good. About the html stuff (we can talk about this later on), I assumed it was up to the receiving server to decide what to do with the parsed info: IE you receive `{content: "*some italicized text*", type: "text/markdown"}, so that server has to get that type and convert it to whatever their DB / front end representation requires.
dessalines merged commit 9707de4d4a into main 2020-11-25 19:54:53 +00:00
nutomic commented 2020-11-26 01:12:46 +00:00
Author
Owner
Copy Link

We could just set the mediaType to text/markdown and call it a day, but then software that doesnt support Markdown might not be able to handle it at all. On the other hand, HTML is the default encoding for content, and presumably supported by most implementations. So by putting both HTML and Markdown into our objects, we ensure maximum compatibility without any negative effect on Lemmy.

Here is an explanation how it works in the client-to-server part of ActivityPub, but I think the same logic applies for server-to-server too.

https://www.w3.org/TR/activitypub/#source-property

We could just set the `mediaType` to `text/markdown` and call it a day, but then software that doesnt support Markdown might not be able to handle it at all. On the other hand, HTML is the default encoding for `content`, and presumably supported by most implementations. So by putting both HTML and Markdown into our objects, we ensure maximum compatibility without any negative effect on Lemmy. Here is an explanation how it works in the client-to-server part of ActivityPub, but I think the same logic applies for server-to-server too. https://www.w3.org/TR/activitypub/#source-property
dessalines commented 2020-11-26 16:43:18 +00:00
Owner
Copy Link

Ohhh okay I see, I didn't realize it was like this:

  "content": "<p>I <em>really</em> like strawberries!</p>",
  "source": {
    "content": "I *really* like strawberries!",
    "mediaType": "text/markdown"}

Ya in that case the converter is fine, as long as lemmy knows to only read the content that's inside the source object.

Ohhh okay I see, I didn't realize it was like this: ``` "content": "<p>I <em>really</em> like strawberries!</p>", "source": { "content": "I *really* like strawberries!", "mediaType": "text/markdown"} ``` Ya in that case the converter is fine, as long as lemmy knows to only read the content that's inside the source object.
nutomic commented 2020-11-26 16:55:02 +00:00
Author
Owner
Copy Link

For now it will keep reading the top-level content from older versions (because they dont set the source). So we will have this in the next Lemmy version, and one version later we can remove the compatibility code (or even later). That means we need to put more focus on announcing new Lemmy versions from now on.

For now it will keep reading the top-level content from older versions (because they dont set the source). So we will have this in the next Lemmy version, and one version later we can remove the compatibility code (or even later). That means we need to put more focus on announcing new Lemmy versions from now on.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: LemmyNet/lemmy#141
No description provided.
Delete Branch "apub-media-type2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?