mikedilger / gossip

Gossip is a nostr client
Other
697 stars 77 forks source link

Wrong rendering of a note's entity included in some code #414

Open dtonon opened 1 year ago

dtonon commented 1 year ago

The "note1xxxx" entity is placed inside a block of code but is still picked up by the inline rendering:

<EventCard id=\"note194n247lecqgcskk5rmmfgrapt4jx7ppq64xec0eca3s4ta3hwkrsex7pxa\" />

Maybe it would make sense to tune the regex, so the entity needs to be included between whitespaces or basic punctuation (full stop, comma, colon, semicolon, ...)?

Screenshot 2023-06-29 alle 12 25 13

nevent1qqsv8pm5vkths45zpypq3s86kgvla32ua6ga0qka07pexvuwej9xsjsppemhxue69uhkummn9ekx7mp0qy2hwumn8ghj7un9d3shjtnyv9kh2uewd9hj79msc23

mikedilger commented 1 year ago

First, since NIP-27 we should not be recognizing any of these nostr URLs unless they start with "nostr:" inside of event content (we should probably also change draft content similarly).

After that, it's very hard to determine which ones to ignore based on characters before and after it.

And lastly, that note clearly didn't render well, the cursor did not advance past the embedded rendered event.

mikedilger commented 1 year ago

I suppose I'll have to update ShatteredContent in nostr-types

dtonon commented 1 year ago

we should not be recognizing any of these nostr URLs unless they start with "nostr:"

Oh true! Forcing this should solve a lot of random cases, without fiddling with the regex.

mikedilger commented 1 year ago

This nostr URL issue was fixed yesterday.

dtonon commented 1 year ago

Great 👍

dtonon commented 1 year ago

@mikedilger maybe the bug reappeared?

image

nevent1qqsqqqqpu2yx32psxfvz4hyjnuqlrq7my8j8ur35jnlxpfs97rtpkpcpremhxue69uhkummnw3ez6ur4vgh8wetvd3hhyer9wghxuet59uq35amnwvaz7tmjv4kxz7fwwajkcmr0wfjx2u3wdejhgtcpz4mhxue69uhhyetvv9ujuerpd46hxtnfduhszrnhwden5te0dehhxtnvdakz7qghwaehxw309aex2mrp0yhxummnw3ezucnpdejz7qg5waehxw309aex2mrp09skymr99ehhyee0qyghwumn8ghj7mn0wd68ytnhd9hx2tcprdmhxue69uhhyetvv9ujummjv9hxwetsd9kxctnyv4mz7m5xgp0

This issue probably is linkable to https://github.com/mikedilger/gossip/issues/348

mikedilger commented 1 year ago

My fix avoided highlighting these when they were not prefixed with "nostr:". This one is prefixed with "nostr:"

Instead of pretending that we can know what the user intends by parsing the characters around it, we should give the user the choice of how to handle these. We might make a pop-up menu on each highlighted link.

mikedilger commented 1 year ago

Reopening. We need a more greedy link finder that recognizes nostr urls too.

dtonon commented 1 year ago

@mikedilger

we should give the user the choice of how to handle these. We might make a pop-up menu on each highlighted link.

This could be a nice option!

We need a more greedy link finder that recognizes nostr urls too.

Indeed, an url seems the only context in which a bad hyperlink to a nostr event could create real usability problems. If we exclude "/" before "nostr:" we should be 99.99% safe and free to link nostr: in any other occurrences.