edemaine / coauthor

Coauthor supercollaboration/discussion forum
MIT License
245 stars 32 forks source link

Internal message links #586

Closed tiborschneider closed 2 years ago

tiborschneider commented 2 years ago

Unfortunately, adding the id attribute to HTML tags in a message does not allow a link to point to that element:

<a href="#ImportantSection">Link to section</a>
<h3 id="ImportantSection">Important Section</h3>

The link works, and appends the #ImportantSection to the url, but the page does not move (neither in HTML mode, nor in Markdown mode). It would be very nice to have working links inside of a message for longer posts.

edemaine commented 2 years ago

The issue is that Coauthor's HTML sanitizer removes id attributes, because (I believe) it's a security risk. Also, multiple messages with the same ids will cause problems, as ids need to be globally unique.

Maybe we could transform id="ImportantSection" to id="messageID_ImportantSection" where messageID is the unique message ID. And if we want to get fancy, we could also autotranslate the link #ImportantSection to #messageID_ImportantSection. (But should allow links to other messages, like #otherMessageID_id, to work.)

It might also be nice to add automatic ids to Markdown sections (## Important Section -> <h2 id="messageID_important-section">Important Section</h2>). I imagine there's a plugin for this.

tiborschneider commented 2 years ago

Thanks for the quick response

I cannot say anything about the security aspect, except that GitHub themselves seems to allow ID attributes in their markdown / html sanitizer, as you can see from this link to the first line.

About the uniqueness, I think if we automatically transform the attributes, we should also do this for the links. However, I also think that it is acceptable if links are not necessarily unique, since as an author, you can always make the link unique.

And yes, there is a plugin for automatically adding the anchors to headings (which is based on markdown-it, which is also what you are using if I understand it correctly).

edemaine commented 2 years ago

Thanks for the GitHub example. This actually confirms that it's a security risk, or at least the risk of breaking the rest of website functionality. If you inspect the source of your message, you'll see it actually turns the id="thanks" into id="user-content-thanks" to avoid conflict with GitHub ids. The href still points to #thanks, so I guess if you accidentally match something in the page, it won't work; and then they must have added a click handler (Coauthor has one too) that looks for user-content-thanks when thanks doesn't exist. Indeed, this link to start-of-content scrolls to the GitHub one (top of page), not the user-content one I added (top of this post).

My proposal is similar, but I think slightly superior, to this. In particular, it's pretty annoying for someone else posting a message with ids to break a message you wrote a while ago, so instead of localizing to user-content- I think it should be localized to the message ID. All of this can be transparent to the user who's just linking within a single message.

edemaine commented 2 years ago

I now really want this feature so that I can inline SVG content with <use> (which requires elements to have ids). I'll probably implement it soon as a result.