Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

Mentions #3

Open
3 tasks done
pfrazee opened this issue Jan 8, 2018 · 29 comments
Open
3 tasks done

Mentions #3

pfrazee opened this issue Jan 8, 2018 · 29 comments

Comments

@pfrazee
Copy link
Member

pfrazee commented Jan 8, 2018

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

  • A UI element for detecting when the user is mentioning somebody during composition.
  • An update to the post JSON which codifies the mentioned people.
  • Add mentions to the notifications index. (I'm in the process of adding this index now.)
@kevinmarks
Copy link
Contributor

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
Copy link
Member Author

pfrazee commented Jan 11, 2018

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
Copy link
Contributor

SaFrMo commented Jan 22, 2018

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
Copy link
Member Author

pfrazee commented Jan 22, 2018

Awesome!

@SaFrMo
Copy link
Contributor

SaFrMo commented Jan 23, 2018

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:

  • adding the ability to remove mentions by deleting the span or clicking an X on that span
  • styling the mention-candidates dropdown
  • fixing the character counter
  • adding to the notifications index
  • formatting frets with mentions
  • submitting to y'all for review!

@pfrazee
Copy link
Member Author

pfrazee commented Jan 23, 2018

Looking good!

@kevinmarks
Copy link
Contributor

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
Copy link
Member Author

pfrazee commented Jan 24, 2018

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
Copy link
Contributor

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
Copy link
Member Author

pfrazee commented Jan 24, 2018

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

@kevinmarks
Copy link
Contributor

kevinmarks commented Jan 24, 2018 via email

@pfrazee
Copy link
Member Author

pfrazee commented Jan 24, 2018

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
Copy link
Contributor

SaFrMo commented Jan 24, 2018

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member Author

pfrazee commented Jan 24, 2018

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
Copy link
Contributor

SaFrMo commented Jan 25, 2018

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:

  • denotes where a mention exists with a @ rather than an offset value (which also allows non-mention uses of a name: Sander Moolin is not a mention, but @Sander Moolin is)
  • allows easy front-end formatting by replacing @${ mention.name } with <a href="${ mention.url }">${ mention.name }</a>, or anything else the app wanted

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
Copy link
Member Author

pfrazee commented Jan 25, 2018

@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
Copy link
Contributor

SaFrMo commented Jan 30, 2018

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
Copy link
Member Author

pfrazee commented Jan 30, 2018 via email

@NicholasGWK
Copy link

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
Copy link

@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
Copy link

The code for that starts around here

@NicholasGWK
Copy link

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
Copy link
Member Author

pfrazee commented Jun 5, 2018

@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
Copy link

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
Copy link
Member Author

pfrazee commented Jun 5, 2018

@NicholasGWK I'm working on the internals still, see dat-ecosystem-archive/DEPs#27 to learn about that

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

@MarkBennett
Copy link

Did you mean to close this issue @pfrazee?

@pfrazee
Copy link
Member Author

pfrazee commented Aug 4, 2018

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants