beakerbrowser / fritter

A peer-to-peer social feed app. (proof of concept)
dat://fritter.hashbase.io
MIT License
363 stars 36 forks source link

Mentions #3

Open pfrazee opened 6 years ago

pfrazee commented 6 years ago

It'd be very nice if we could mention other users. This will require:

kevinmarks commented 6 years ago

The challenge with using @ is that there are no handles in Fritter, so it would need to be more like the way facebook completes than how twitter does. Also, URL tags are going to be challenging with dat URLs. Some thinking on this here: https://indieweb.org/person-tag

pfrazee commented 6 years ago

The challenge with using @ is that there are no handles in Fritter, so it would need to be more like the way facebook completes than how twitter does.

Yeah probably

Also, URL tags are going to be challenging with dat URLs. Some thinking on this here: https://indieweb.org/person-tag

Is the person-tag an HTML based spec? All of fritter's data is published as JSON. What we really need to do is switch to JSON-LD and adopt one of the popular vocabs (like activitypub).

SaFrMo commented 6 years ago

I started sketching out some implementation here - whenever the user enters @, Fritter will start looking for profiles they follow whose name matches the text following the @ (Facebook-style like @kevinmarks mentioned, going by a given name as opposed to a handle).

Next, I can add the array of mentioned users/their Dat URLs to the post, work on a way to remove a mention from a post (maybe a tag-style border around the mention with an "X" to remove?), and add to the notifications index.

Let me know if there are any thoughts on the way this is going so far, otherwise I'll keep at it in the coming week!

pfrazee commented 6 years ago

Awesome!

SaFrMo commented 6 years ago

screen shot 2018-01-23 at 10 32 54 am

The big change so far is that the post input is now a div with contenteditable set to true. I looked around at a few other tagging/mentions systems like Twitter, WordPress, and Stack Overflow, and this seems like the way to go.

I'm also using a modified version of LibFritter that accepts mentions as a parameter. The way I have it set up there is pretty bare-bones, so we should talk about what it should look like (or if we need it at all) when it comes time to implement. Frets with mentions under that schema look like this:

screen shot 2018-01-23 at 10 41 39 am

Roadmap from here is:

pfrazee commented 6 years ago

Looking good!

kevinmarks commented 6 years ago

The proposed structure here feels like a step towards the dreaded twitter entities model. At least you have the text to match rather than offsets into it though. But having an actual HTML fragment for this might be clearer, especially fi you want to support other contenteditable features in future.

pfrazee commented 6 years ago

That's a fair point. In SSB we used Markdown to solve the issue. If we use HTML, we have sanitation issues though, right? CSPs help with that sort of thing but it still makes me nervous. WDYT?

kevinmarks commented 6 years ago

Markdown is a "now you have 2 problems" kind of answer. You have sanitation issues wherever you are constructing HTML from text - if I put a javascript: url in Sander's data structure above, or find a way to break out of the enclosing " in the generated html, I can still mess with your page. (Ask me about what I had to do to make svgshare.com behave sometime).

pfrazee commented 6 years ago

Sanitation is much easier if you're not expecting html elements to be specified in the input though, right?

kevinmarks commented 6 years ago

The best way to sanitize is to parse with a real html parser, then edit the resulting dom to remove any elements you don't allow (or keep only the ones that you allow). How browsers handle svg in an img tag is an example, iframe sandbox is another. Beaker has a very good html parser available, leverage that.

On 24 Jan 2018 16:17, "Paul Frazee" notifications@github.com wrote:

Sanitation is much easier if you're not expecting html elements to be specified in the input though, right?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/beakerbrowser/fritter/issues/3#issuecomment-360187472, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGCwFPEm9brZWGC4EaxKxCNzxtlpOXyks5tN1e2gaJpZM4RWsIH .

pfrazee commented 6 years ago

The simplest to implement and therefore safest approach is to create a container element and then set the content using .textContent (more info). By introducing HTML in the content we force all apps to apply tag & attribute whitelists which is harder to get right.

I'm not saying that means we absolutely shouldn't use HTML, but it's an important tradeoff that we should consider.

SaFrMo commented 6 years ago

Great point on the entities model @kevinmarks. Just to summarize and make sure I'm understanding correctly, the fret from above:

{
  "text":"Sander Moolin is working on mentions!",
  "createdAt":1516721258354,
  "mentions": [
    {
      "name":"Sander Moolin",
      "url":"dat://14e02bfbe6d66113327a1e2f473dcd639dc3d9d97a05c1e5778a6c295fd02095"
    }
  ]
}

could instead look like this:

{
  "text": "<span class="mention" data-url="dat://14e02bfbe6d66113327a1e2f473dcd639dc3d9d97a05c1e5778a6c295fd02095">Sander Moolin</span> is working on mentions!",
  "createdAt":1516721258354
}

Then it's a matter of filtering out undesired tags, attributes, values, etc. like @pfrazee mentioned, which can be handled on the Fritter feed when rendering posts (and any other apps that can read Fritter posts).

I really like the idea of storing fret text as HTML - it keeps things much more flexible, even if it does have the increased security requirements that Paul mentioned. I'd be willing to spend some more time on this than originally planned and document allowed tags & attributes, if that was a helpful factor. Thoughts?

kevinmarks commented 6 years ago

Well, I'd advocate for the full indeweb person tag syntax, so { "text": "<a class="u-category h-card" href="dat://14e02bfbe6d66113327a1e2f473dcd639dc3d9d97a05c1e5778a6c295fd02095">Sander Moolin</a> is working on mentions!", "createdAt":1516721258354 }

If you want some good prior art on sanitizing html from unknown sources, I recommend feedparser or if you want a full js+css+html approach, Caja which is on npm here

kevinmarks commented 6 years ago

This test is an interesting one: https://news.ycombinator.com/item?id=11446984 Fritter passes 2, but 1 is less clear - not sure if it is quite at the tag/attribute distinction yet.

pfrazee commented 6 years ago

We're stacking in a lot of new complexity here. Not only is security more complex to handle, but you're putting data into a second internal format. The mention <a> or <span> is encoding data that was previously placed in the mentions attribute of the JSON object. That means that in order to properly index the mentions, the applications are going to have to parse HTML after parsing JSON. There are many simpler alternatives. Heck, specifying character offsets into the text string is simpler than that.

We should only be considering a general markup like HTML in the text if we're really sure that we need that flexibility, and then we need to consider rules around presentation, semantics, fallback behaviors, and etc.

SaFrMo commented 6 years ago

Really good points on nesting data formats @pfrazee! While HTML itself might be fine in a post's content (based on its use in the ActivityPub spec, I can definitely see how that extra level of data being encoded would be overly complicated.

Based on that and on Fritter's purpose as a microblogging platform, it does makes more sense not to include the extra flexibility of general markup like HTML. (I forget that this is all decentralized web, after all, and if someone wanted the flexibility of HTML, they're literally three clicks away from creating and hosting it themselves!)

So to do a complete 180 😝, we could make a fret look like this:

{
  "text": "@Sander Moolin is working on mentions!",
  "createdAt": 1516721258354,
  "mentions": [
    "name": "Sander Moolin",
    "url": "dat://14e02bfbe6d66113327a1e2f473dcd639dc3d9d97a05c1e5778a6c295fd02095"
  ]
}

This way, the fret:

It's still got the separate mentions property that makes it feel like a Twitter entity, but I don't see a way around that without encoding the data in HTML - definitely open to suggestions though!

I also thought that using <span>Sander Moolin</span> instead of @Sander Moolin in the text might work, but the @ is a bit more concise and doesn't make any markup assumptions.

pfrazee commented 6 years ago

@SaFrMo I think that's good. It also lets non-supporting clients know that a mention was attempted without putting too much junk in the text.

SaFrMo commented 6 years ago

Almost there! Here it is in action using a couple test accounts: screen shot 2018-01-30 at 10 15 19 am

Just need to format mentions in frets, double-check the PR code, and submit!

pfrazee commented 6 years ago

Sweet!

On Tue, Jan 30, 2018 at 9:17 AM, Sander Moolin notifications@github.com wrote:

Almost there! Here it is in action using a couple test accounts: [image: screen shot 2018-01-30 at 10 15 19 am] https://user-images.githubusercontent.com/5344587/35573963-8dbe2db4-05a6-11e8-9b21-ac858c218d8c.png

Just need to format mentions in frets, double-check the PR code, and submit!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/beakerbrowser/fritter/issues/3#issuecomment-361625139, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNhUx8KF1l8uZBohkbp41q5dNDDaaUEks5tPzJsgaJpZM4RWsIH .

NicholasGWK commented 6 years ago

Awesome project/feature! I am really new to P2P and super confused about how notifications would work...can someone do an ELI5? I tried peering through the source/browsing the archive on Beaker but no success 😬

RangerMauve commented 6 years ago

@NicholasGWK By notifications, do you mean push notifications, or the notifications tab in fritter?

If it's the former, that's not implemented yet.

The latter works by listening on changes in your peers' dats and when it sees a new message mentioning you, it creates the notification.

RangerMauve commented 6 years ago

The code for that starts around here

NicholasGWK commented 6 years ago

Thanks @RangerMauve, dug into source + more articles and its making sense now! I was referring to the notifications tab where it hits all people you're following. Cheers

pfrazee commented 6 years ago

@NicholasGWK I'm working on some new data-channel APIs in Beaker to make pinging (some) peers possible, even if they don't follow you. We'll see how well they work.

NicholasGWK commented 6 years ago

Awesome @pfrazee ! Would love to read up on that if possible or get some more context; just discovered Beaker/Dat after JSConfg and am really excited about it and trying to build stuff/contribute! Cheers

pfrazee commented 6 years ago

@NicholasGWK I'm working on the internals still, see https://github.com/datprotocol/DEPs/pull/27 to learn about that

(Closing this issue since mentions were added, but feel free to keep discussion)

MarkBennett commented 6 years ago

Did you mean to close this issue @pfrazee?

pfrazee commented 6 years ago

Yep, > (Closing this issue since mentions were added, but feel free to keep discussion)