fraction / oasis

Free, open-source, peer-to-peer social application that helps you follow friends and discover new ones on Secure Scuttlebutt (SSB).
http://oasis-demo.fraction.io
GNU Affero General Public License v3.0
287 stars 42 forks source link

Cannot post a mention-like text without it actually becoming a mention #466

Open black-puppydog opened 3 years ago

black-puppydog commented 3 years ago

What's the problem you want solved?

This happened in %zfgm5l/INLI9Nggn7hzniQjzr2mr2aC9korQ/UpARTQ=.sha256: I tried to post this text:

daan@hermes ~/code/patchwork πŸ¦†127Β» npm ci                                                                                                                                                        ξ‚  master|✚1 16:46
npm ERR! code 1

The mentions flow made it into this:

daan[@hermes](@hoSvAN2VxIumAjQp1jY7F4ZBujATFuKOgBtYEIXJc6o=.ed25519) ~/code/patchwork πŸ¦†127Β» npm ci                                                                                                                                                        ξ‚  master|✚1 16:46
npm ERR! code 1

There are IMO two issues with this, but I think they may be worth thinking about together first, and breaking them up later:

  1. This was clearly a false-positive. Arguably, code fragments rarely need mentions.
  2. I noticed this before posting, but the UI didn't actually allow me to fix it by hand. I removed the matched mention, but then "Preview" re-introduced it. So I was left with hitting "Publish" which then published the message with the wrong mention.

Is there a solution you'd like to recommend?

  1. Don't match mentions in code (inline or block)
  2. Provide some way of previewing/publishing a hand-fixed version.
cblgh commented 3 years ago

1 is very doable (one approach: refine regex to not match inline code, temporarily splice out code blocks from text & reintroduce after matching mentions)

2 i have no suggestions for / feels difficult

i probably won't get to this for a while, so anyone else is most certainly free to prod it πŸ–€

christianbundy commented 3 years ago

Another take on 1:

  1. Markdown -> HTML -> DOM (jsdom/etc)
  2. Walk the DOM and make substitutions, avoid making substitutions inside code or pre (or img src or whatever)
  3. DOM -> HTML -> Markdown
christianbundy commented 3 years ago

Another option: weirder syntax.

Example: <@christian>

Sure, this isn't a Good Solution in that it doesn't solve for <pre> or <code> elements, but it's also a hell of a lot less likely to happen randomly. What do y'all think? :man_shrugging:

christianbundy commented 3 years ago

I'd really love to get this quickfixed before the next release, what do y'all think the easiest solution is here? Also, does this mean that email addresses get replaced?

christianbundy commented 3 years ago

Maybe also useful: how big of a problem do others think this is? If I had known this during review I don't think I would've merged the PR, but reverting it feels bad. Do others think this is small enough that we should release it to the public in this state, or should it be fixed, or what should we do? I don't like Oasis having a big bug and not having any energy to fix it. :/

black-puppydog commented 3 years ago

I think oasis is much better with the last pr than without it.

This will come back to bite for emails and other spurious things.

Personally I'd prefer a way of "freezing" mention substitution so I can edit until I'm happy with the post, then preview one last time without mention magic, then send it off. But an extra checkbox might make the posting form a bit clunky. Might be down to presentation and good UI strings... :thinking:

christianbundy commented 3 years ago

Maybe when you're in the preview page the publish button should be "publish exactly what I just said with no substitutions"? I'm honestly kinda fond of requiring @@mention or @[mention] or similar, but I understand that's also kinda hacky as shit.

christianbundy commented 3 years ago

(I actually really like <@mention> since <> is an explicit Markdown link and it should never coincide with emails/etc.)