Punctuation at end of URL in comments incorporated into link, breaking the link

[ebarry]

Posting for BHamster:

Please describe the problem

What happened just before the problem occurred?

When making a comment on a post, if people paste a whole URL at the end of a sentence and put a period at the end of the URL, the hyperlink that automatically generates in the published comment incorporates the period and then becomes a bad link. e.g., this recent comment.

What did you expect to see that you didn't?
I would expect that our editor is smart enough not to add the period or comma to the end of a link.
Or perhaps this expectation is too high, because it is actually quite difficult to achieve this? Perhaps corporate social media platforms have spent a lot of money to stop this from happening?

Please show us where to look

https://publiclab.org/questions/bhamster/02-10-2022/is-there-federal-level-recourse-that-communities-can-take-when-mine-reclamation-is-not-being-done-properly#c29860

Thank you!

[jywarren]

Thank you! I think we should be able to address this by tweaking the markdown interpreter. The comment body is rendered here:

plots2/app/views/notes/_comment.html.erb

Lines 146 to 149 in 7c3af44

	<% comment_body = comment.render_body %>
	<div class="comment-body" id="comment-body-<%= comment.cid %>">
	<%= raw insert_extras(filtered_comment_body(comment_body)) %>
	</div>

But it's parsed from markdown here:

plots2/app/models/comment.rb

Lines 505 to 524 in ecae3b9

	def render_body
	body = RDiscount.new(
	title_suggestion(self),
	:autolink
	).to_html
	# if it has quoted email text that wasn't caught by the yahoo and gmail filters,
	# manually insert the comment filter delimeter:
	parsed = parse_quoted_text
	if !trimmed_content? && parsed.present?
	body = parsed[:body] + COMMENT_FILTER + parsed[:boundary] + parsed[:quote]
	end
	allowed_tags = %w(a acronym b strong i em li ul ol h1 h2 h3 h4 h5 h6 blockquote br cite sub sup ins p iframe del hr img input code table thead tbody tr th td span dl dt dd div)
	# Sanitize the HTML (remove malicious attributes, unallowed tags...)
	sanitized_body = ActionController::Base.helpers.sanitize(body, tags: allowed_tags)
	# Properly parse HTML (close incomplete tags...)
	Nokogiri::HTML::DocumentFragment.parse(sanitized_body).to_html
	end

Yes, so the RDiscount markdown parser we use does the same thing:

RDiscount.new("the BLM Surface Management Handbook: https://www.blm.gov/sites/blm.gov/files/H-3809-1.pdf.",:autolink).to_html
 => "<p>the BLM Surface Management Handbook: <a href=\"https://www.blm.gov/sites/blm.gov/files/H-3809-1.pdf.\">https://www.blm.gov/sites/blm.gov/files/H-3809-1.pdf.</a></p>\n" 

Other systems are affected but GFM says Trailing punctuation (specifically, ?, !, ., ,, :, *, _, and ~) will not be considered part of the autolink, though they may be included in the interior of the link:

Commonmark doesn't address the issue. Other platforms have had to make this fix too and even in earlier versions of Ruby.

Also noting this is only affecting comments, not wikis; see the link to the Gitter chatroom at https://publiclab.org/wiki/gsoc-ideas#Get+in+touch where the URL is correctly parsed.

I think this is because we don't use RDiscount's auto_link for nodes; we use Ruby's (fixed) auto_link:

plots2/app/views/notes/show.html.erb

Line 76 in 7c3af44

<%= raw auto_link(insert_extras(body), :sanitize => false) %>

Let's try the same for comments. That means we remove the :autolink on this line and we wrap the following line in auto_link:

plots2/app/views/notes/_comment.html.erb

Line 148 in 7c3af44

<%= raw insert_extras(filtered_comment_body(comment_body)) %>

So it would look like this:

raw auto_link(insert_extras(filtered_comment_body(comment_body)), :sanitize => false)

This could be a good FTO, but we have to watch out for whether it breaks any tests, as it's a pretty obscure change!

---

ALSO: Noting that this could come up again potentially if the React comment renderer is different? @noi5e do you think this might be an issue?

---

This has been marked as a good candidate for becoming a first-timers-only issue like these, meaning that it's simple, self-contained, and with some extra formatting, could be a great entry point for a new contributor. If you're familiar enough with this code, please consider reformatting or reposting it as a first-timers-only issue, and then ping @publiclab/reviewers to get it labelled. Or, if this is not your first time, try to solve it yourself!

---

[PeculiarE]

ALSO: Noting that this could come up again potentially if the React comment renderer is different? @noi5e do you think this might be an issue?

So I decided to implement the fix on my local branch just to visually check out the effects on GitPod. Links no longer break but it looks like the React comment renderer is being affected. Links don't appear as hyperlinks once the react flag is set to true. Everything is rendered as normal text. I've attached a short demo.

react-comment-renderer-demo.1.mp4

This could be a good FTO, but we have to watch out for whether it breaks any tests, as it's a pretty obscure change!

Wasn't able to verify if the change was breaking any test on my local. I'm using sqlite3 for my db and so, not all my tests pass on a normal day 😞.

[jywarren]

Excellent digging, @PeculiarE! And thorough testing, thank you!

Hmm, ok, so I think what's happening is that we removed autolink from early in the process, and added it AFTER the react and non-react processes diverge. So possibly it is now working in non-react, but we have to find the corresponding spot where the react version needs an autolink step added.

We may need some help from @noi5e to track where that is. I can see comment body templates in React are here: https://github.com/publiclab/plots2/blob/main/app/javascript/components/CommentDisplay.js

Could the spot to re-introduce autolink be here?

plots2/app/helpers/comment_helper.rb

Line 28 in 5ec93e0

comment_json[:htmlCommentText] = raw insert_extras(filtered_comment_body(comment.render_body))

[PeculiarE]

Thank you @jywarren ❤️

So you were right on on the correct spot to reintroduce autolink to the react version. Tested it on GitPod and the hyperlinks rendered properly on the React system. 🚀 🎉...please see the demo below

react-comment-renderer-demo.2.mp4

I believe this fixes the issue completely on both commenting systems. May I go ahead and add it to the open FTO issue at #10947?

[noi5e]

Excellent work @PeculiarE! Yes, feel free to add this to the open FTO you mentioned, or even create a new one.

[PeculiarE]

Thank you ❤️ @noi5e! I've created a new FTO issue for this..

[PeculiarE]

Hi @jywarren, since the two FTOs (and the resulting PRs) created to resolve this issue have been successfully merged and closed, can we consider this issue closed?

[jywarren]

Yes sounds good!! Thank you!